[Sema] Resolve placeholder types before type deduction to silence spurious `-Warc-repeated-use-of-weak` warnings

[ahatanak]

Summary

The spurious -Warc-repeated-use-of-weak warnings are issued when an initializer expression uses a weak ObjC pointer.

My first attempt to silence the warnings (r350917) caused clang to reject code that is legal in C++17. The patch is based on the feedback I received from Richard when the patch was reverted.

http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20190422/268945.html
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20190422/268943.html

[rjmccall]

lib/Sema/SemaDecl.cpp

11123

if (Init->hasPlaceholderType(BuiltinType::PseudoObject))

Although maybe this should just be isNonOverloadPlaceholderType(), since it's the syntactic-only placeholders that we're trying to cover. (Feel free to add Expr::hasNonOverloadPlaceholderType() for convenience.)

[ahatanak]

Address review comments.

lib/Sema/SemaDecl.cpp

11123

I had to move the code in deduceVarTypeFromInitializer that converts UnknownAnyType to id. The conversion has to happen before CheckPlaceholderExpr is called.

[rjmccall]

lib/Sema/SemaDecl.cpp

11123

Seems reasonable. Can you do the cleanup I requested?

[ahatanak]

Add function Expr::hasNonOverloadPlaceholderType.

[rjmccall]

Thanks, looks much better.

include/clang/Sema/Sema.h

2066

A shame this needs to be propagated down like this. Is there a reasonable way to just mark on the declaration that we defaulted the type, like an unparseable attribute? This might be interesting to other clients ultimately.

[ahatanak]

Add an implicit-only attribute ObjCDefaultedAnyToId to avoid passing the flag down.

[rjmccall]

Thanks, LGTM.

[rjmccall]

include/clang/Basic/Attr.td

1844 ↗

(On Diff #207931)

Oh, please add a comment on this explaining what it means, since it's not based on a language feature.

include/clang/AST/Expr.h

public:
   /// Returns whether this expression has a specific placeholder type.
   bool hasPlaceholderType(BuiltinType::Kind K) const {
     assert(BuiltinType::isPlaceholderTypeKind(K));
     if (const BuiltinType *BT = dyn_cast<BuiltinType>(getType()))
       return BT->getKind() == K;
     return false;
   }
 
+  /// Returns whether this expression has a placeholder type that isn't
+  /// Overload.
+  bool hasNonOverloadPlaceholderType() const {
+    if (auto *PlaceholderType = getType()->getAsPlaceholderType())
+      if (PlaceholderType->isNonOverloadPlaceholderType())
+        return true;
+    return false;
+  }
+
   /// isKnownToHaveBooleanValue - Return true if this is an integer expression
   /// that is known to return 0 or 1.  This happens for _Bool/bool expressions
   /// but also int expressions which are produced by things like comparisons in
   /// C.
   bool isKnownToHaveBooleanValue() const;
 
   /// isIntegerConstantExpr - Return true if this expression is a valid integer
   /// constant expression, and, if so, return its value in Result.  If not a

include/clang/Sema/Sema.h

NamedDecl *ActOnVariableDeclarator(Scope *S, Declarator &D, DeclContext *DC,
                                      ArrayRef<BindingDecl *> Bindings = None);
   NamedDecl *
   ActOnDecompositionDeclarator(Scope *S, Declarator &D,
                                MultiTemplateParamsArg TemplateParamLists);
   // Returns true if the variable declaration is a redeclaration
   bool CheckVariableDeclaration(VarDecl *NewVD, LookupResult &Previous);
   void CheckVariableDeclarationType(VarDecl *NewVD);
   bool DeduceVariableDeclarationType(VarDecl *VDecl, bool DirectInit,
-                                     Expr *Init);
+                                     Expr *Init, bool DefaultedAnyToId);
   void CheckCompleteVariableDeclaration(VarDecl *VD);
   void CheckCompleteDecompositionDeclaration(DecompositionDecl *DD);
   void MaybeSuggestAddingStaticToDecl(const FunctionDecl *D);
 
   NamedDecl* ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                                      TypeSourceInfo *TInfo,
                                      LookupResult &Previous,
                                      MultiTemplateParamsArg TemplateParamLists,

 
   QualType DeduceTemplateSpecializationFromInitializer(
       TypeSourceInfo *TInfo, const InitializedEntity &Entity,
       const InitializationKind &Kind, MultiExprArg Init);
 
   QualType deduceVarTypeFromInitializer(VarDecl *VDecl, DeclarationName Name,
                                         QualType Type, TypeSourceInfo *TSI,
                                         SourceRange Range, bool DirectInit,
-                                        Expr *Init);
+                                        Expr *Init, bool DefaultedAnyToId);
 
   TypeLoc getReturnTypeLoc(FunctionDecl *FD) const;
 
   bool DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
                                         SourceLocation ReturnLoc,
                                         Expr *&RetExpr, AutoType *AT);
 
   FunctionTemplateDecl *getMoreSpecializedTemplate(FunctionTemplateDecl *FT1,

lib/Sema/SemaDecl.cpp

struct VarDeclOrName {
     }
   };
 } // end anonymous namespace
 
 QualType Sema::deduceVarTypeFromInitializer(VarDecl *VDecl,
                                             DeclarationName Name, QualType Type,
                                             TypeSourceInfo *TSI,
                                             SourceRange Range, bool DirectInit,
-                                            Expr *Init) {
+                                            Expr *Init, bool DefaultedAnyToId) {
   bool IsInitCapture = !VDecl;
   assert((!VDecl || !VDecl->isInitCapture()) &&
          "init captures are expected to be deduced prior to initialization");
 
   VarDeclOrName VN{VDecl, Name};
 
   DeducedType *Deduced = Type->getContainedDeducedType();
   assert(Deduced && "deduceVarTypeFromInitializer for non-deduced type");
QualType Sema::deduceVarTypeFromInitializer(VarDecl *VDecl,
   if (DirectInit && isa<InitListExpr>(DeduceInit)) {
     Diag(Init->getBeginLoc(), IsInitCapture
                                   ? diag::err_init_capture_paren_braces
                                   : diag::err_auto_var_init_paren_braces)
         << isa<InitListExpr>(Init) << VN << Type << Range;
     return QualType();
   }
 
-  // Expressions default to 'id' when we're in a debugger.
-  bool DefaultedAnyToId = false;
-  if (getLangOpts().DebuggerCastResultToId &&
-      Init->getType() == Context.UnknownAnyTy && !IsInitCapture) {
-    ExprResult Result = forceUnknownAnyToType(Init, Context.getObjCIdType());
-    if (Result.isInvalid()) {
-      return QualType();
-    }
-    Init = Result.get();
-    DefaultedAnyToId = true;
-  }
-
   // C++ [dcl.decomp]p1:
   //   If the assignment-expression [...] has array type A and no ref-qualifier
   //   is present, e has type cv A
   if (VDecl && isa<DecompositionDecl>(VDecl) &&
       Context.hasSameUnqualifiedType(Type, Context.getAutoDeductType()) &&
       DeduceInit->getType()->isConstantArrayType())
     return Context.getQualifiedType(DeduceInit->getType(),
                                     Type.getQualifiers());
if (!inTemplateInstantiation() && !DefaultedAnyToId && !IsInitCapture &&
     SourceLocation Loc = TSI->getTypeLoc().getBeginLoc();
     Diag(Loc, diag::warn_auto_var_is_id) << VN << Range;
   }
 
   return DeducedType;
 }
 
 bool Sema::DeduceVariableDeclarationType(VarDecl *VDecl, bool DirectInit,
-                                         Expr *Init) {
+                                         Expr *Init, bool DefaultedAnyToId) {
   QualType DeducedType = deduceVarTypeFromInitializer(
       VDecl, VDecl->getDeclName(), VDecl->getType(), VDecl->getTypeSourceInfo(),
-      VDecl->getSourceRange(), DirectInit, Init);
+      VDecl->getSourceRange(), DirectInit, Init, DefaultedAnyToId);
   if (DeducedType.isNull()) {
     VDecl->setInvalidDecl();
     return true;
   }
 
   VDecl->setType(DeducedType);
   assert(VDecl->isLinkageValid());
 
if (VDecl->getType()->isUndeducedType()) {
     // be deduced based on the chosen correction if the original init contains a
     // TypoExpr.
     ExprResult Res = CorrectDelayedTyposInExpr(Init, VDecl);
     if (!Res.isUsable()) {
       RealDecl->setInvalidDecl();
       return;
     }
     Init = Res.get();
+    bool DefaultedAnyToId = false;
+
+    // Expressions default to 'id' when we're in a debugger
+    // and we are assigning it to a variable of Objective-C pointer type.
+    if (getLangOpts().DebuggerCastResultToId &&
+        Init->getType() == Context.UnknownAnyTy) {
+      ExprResult Result = forceUnknownAnyToType(Init, Context.getObjCIdType());
+      if (Result.isInvalid()) {
+        VDecl->setInvalidDecl();
+        return;
+      }
+      Init = Result.get();
+      DefaultedAnyToId = true;
+    } else if (!Init->getType().isNull() &&
+               Init->hasNonOverloadPlaceholderType()) {
+      Res = CheckPlaceholderExpr(Init).get();
+      if (!Res.isUsable()) {
+        VDecl->setInvalidDecl();
+        return;
+      }
+      Init = Res.get();
+    }
 
-    if (DeduceVariableDeclarationType(VDecl, DirectInit, Init))
+    if (DeduceVariableDeclarationType(VDecl, DirectInit, Init,
+                                      DefaultedAnyToId))
       return;
   }
 
   // dllimport cannot be used on variable definitions.
   if (VDecl->hasAttr<DLLImportAttr>() && !VDecl->isStaticDataMember()) {
     Diag(VDecl->getLocation(), diag::err_attribute_dllimport_data_definition);
     VDecl->setInvalidDecl();
     return;
   }
if (VarDecl *Var = dyn_cast<VarDecl>(RealDecl)) {
     // C++1z [dcl.dcl]p1 grammar implies that an initializer is mandatory.
     if (isa<DecompositionDecl>(RealDecl)) {
       Diag(Var->getLocation(), diag::err_decomp_decl_requires_init) << Var;
       Var->setInvalidDecl();
       return;
     }
 
     if (Type->isUndeducedType() &&
-        DeduceVariableDeclarationType(Var, false, nullptr))
+        DeduceVariableDeclarationType(Var, false, nullptr, false))
       return;
 
     // C++11 [class.static.data]p3: A static data member can be declared with
     // the constexpr specifier; if so, its declaration shall specify
     // a brace-or-equal-initializer.
     // C++11 [dcl.constexpr]p1: The constexpr specifier shall be applied only to
     // the definition of a variable [...] or the declaration of a static data
     // member.

lib/Sema/SemaExpr.cpp

Sema::MaybeConvertParenListExprToParenExpr(Scope *S, Expr *OrigExpr) {
   if (Result.isInvalid()) return ExprError();
 
   return ActOnParenExpr(E->getLParenLoc(), E->getRParenLoc(), Result.get());
 }
 
 ExprResult Sema::ActOnParenListExpr(SourceLocation L,
                                     SourceLocation R,
                                     MultiExprArg Val) {
+  for (size_t I = 0, E = Val.size(); I != E; ++I)
+    if (Val[I]->hasNonOverloadPlaceholderType()) {
+      ExprResult Result = CheckPlaceholderExpr(Val[I]);
+      if (!Result.isUsable())
+        return ExprError();
+      Val[I] = Result.get();
+    }
+
   return ParenListExpr::Create(Context, L, Val, R);
 }
 
 /// Emit a specialized diagnostic when one expression is a null pointer
 /// constant and the other is not a pointer.  Returns true if a diagnostic is
 /// emitted.
 bool Sema::DiagnoseConditionalForNull(Expr *LHSExpr, Expr *RHSExpr,
                                       SourceLocation QuestionLoc) {

lib/Sema/SemaExprCXX.cpp

Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
   Expr **Inits = &Initializer;
   unsigned NumInits = Initializer ? 1 : 0;
   if (ParenListExpr *List = dyn_cast_or_null<ParenListExpr>(Initializer)) {
     assert(initStyle == CXXNewExpr::CallInit && "paren init for non-call init");
     Inits = List->getExprs();
     NumInits = List->getNumExprs();
   }
 
+  for (unsigned I = 0, E = NumInits; I != E; ++I)
+    if (Inits[I]->hasNonOverloadPlaceholderType()) {
+      ExprResult Result = CheckPlaceholderExpr(Inits[I]);
+      if (!Result.isUsable())
+        return ExprError();
+      Inits[I] = Result.get();
+    }
+
   // C++11 [expr.new]p15:
   //   A new-expression that creates an object of type T initializes that
   //   object as follows:
   InitializationKind Kind
       //     - If the new-initializer is omitted, the object is default-
       //       initialized (8.5); if no initialization is performed,
       //       the object has indeterminate value
       = initStyle == CXXNewExpr::NoInit

lib/Sema/SemaLambda.cpp

if (Init->containsUnexpandedParameterPack()) {
       // diagnose this later when we try to capture it.
     }
   }
   TypeSourceInfo *TSI = TLB.getTypeSourceInfo(Context, DeductType);
 
   // Deduce the type of the init capture.
   QualType DeducedType = deduceVarTypeFromInitializer(
       /*VarDecl*/nullptr, DeclarationName(Id), DeductType, TSI,
-      SourceRange(Loc, Loc), IsDirectInit, Init);
+      SourceRange(Loc, Loc), IsDirectInit, Init, false);
   if (DeducedType.isNull())
     return QualType();
 
   // Are we a non-list direct initialization?
   ParenListExpr *CXXDirectInit = dyn_cast<ParenListExpr>(Init);
 
   // Perform initialization analysis and ensure any implicit conversions
   // (such as lvalue-to-rvalue) are enforced.

test/SemaObjC/arc-repeated-weak.mm

-// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -std=c++11 -Warc-repeated-use-of-weak -verify %s
-// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++11 -Warc-repeated-use-of-weak -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -std=c++14 -Warc-repeated-use-of-weak -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++14 -Warc-repeated-use-of-weak -verify %s
 
 @interface Test {
 @public
   Test *ivar;
   __weak id weakIvar;
 }
 @property(weak) Test *weakProp;
 @property(strong) Test *strongProp;
void foo() {
   NSString * t2 = NSBundle.foo2.prop;
   use(NSBundle.foo2.weakProp); // expected-warning{{weak property 'weakProp' may be accessed multiple times}}
   use(NSBundle2.foo2.weakProp); // expected-note{{also accessed here}}
   decltype([NSBundle2.foo2 weakProp]) t3;
   decltype(NSBundle2.foo2.weakProp) t4;
   __typeof__(NSBundle2.foo2.weakProp) t5;
 }
 
+void testAuto() {
+  auto __weak wp = NSBundle2.foo2.weakProp;
+}
+
+void testLambdaCaptureInit() {
+  [capture(NSBundle2.foo2.weakProp)] {} ();
+}
+
+void testAutoNew() {
+  auto p = new auto(NSBundle2.foo2.weakProp);
+}
+
 // This used to crash in the constructor of WeakObjectProfileTy when a
 // DeclRefExpr was passed that didn't reference a VarDecl.
 
 typedef INTF * INTFPtrTy;
 
 enum E {
   e1
 };