[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D55662#1336835 <https://reviews.llvm.org/D55662#1336835>, @ahatanak wrote:

> In D55662#1335773 <https://reviews.llvm.org/D55662#1335773>, @rjmccall wrote:
>
> > Okay.  You may need to push an unevaluated context when doing that.
>
>
> Since I'm just moving the call to `CheckPlaceholderExpr` to the call site, I don't think I have to push an unevaluated context there?


Hmm.  Right, for the `auto` inference specifically it's fine because the expression is in fact evaluated: we're not just eliminating placeholders in order to resolve `decltype`, we're eliminating placeholders to actually figure out what's going on with the initialization.

> Also, it looks like I can just change the check `Init->getType()->isNonOverloadPlaceholderType()`  at the beginning of `Sema::DeduceAutoType` to `Init->getType()->getAsPlaceholderType()` instead of inserting the call to `CheckPlaceholderExpr` right before the call to `BuildDecltypeType`.

The reason it's specifically checking for a non-overload placeholder is that normal `auto` can have additional structure in the declarator that could potentially resolve an overload set without picking a single template specialization.  So you can only check for an arbitrary placeholder if this is the `decltype(auto)` case.  But I agree that it still makes sense to only do this once up front, yeah.



================
Comment at: lib/Sema/SemaType.cpp:8043
 QualType Sema::BuildTypeofExprType(Expr *E, SourceLocation Loc) {
-  ExprResult ER = CheckPlaceholderExpr(E);
-  if (ER.isInvalid()) return QualType();
-  E = ER.get();
+  assert(!E->getType()->getAsPlaceholderType() && "unexpected placeholder");
 
----------------
There is a `isPlaceholderType()` method that looks a little cleaner than a `getAs` call.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55662/new/

https://reviews.llvm.org/D55662