[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

11101

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

11101

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

11101

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 ↗

(On Diff #207166)

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.

lib/Sema/SemaDecl.cpp

if (VDecl->getType()->isUndeducedType()) {
     // TypoExpr.
     ExprResult Res = CorrectDelayedTyposInExpr(Init, VDecl);
     if (!Res.isUsable()) {
       RealDecl->setInvalidDecl();
       return;
     }
     Init = Res.get();
 
+    if (!Init->getType().isNull())
+      if (auto *placeholderType = Init->getType()->getAsPlaceholderType())
+        if (placeholderType->getKind() == BuiltinType::PseudoObject) {
+          Res = CheckPlaceholderExpr(Init).get();
+          if (!Res.isUsable())
+            return;
+          Init = Res.get();
+        }
+
     if (DeduceVariableDeclarationType(VDecl, DirectInit, Init))
       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();

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 (auto *PlaceholderType = Val[I]->getType()->getAsPlaceholderType())
+      if (PlaceholderType->getKind() == BuiltinType::PseudoObject) {
+        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 (auto *PlaceholderType = Inits[I]->getType()->getAsPlaceholderType())
+      if (PlaceholderType->getKind() == BuiltinType::PseudoObject) {
+        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

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
 };