[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin marked 3 inline comments as done.
cor3ntin added inline comments.


================
Comment at: clang/lib/AST/Decl.cpp:1831
+  // Objective-C as an extension.
+  if (isa<ParmVarDecl>(this) && getLangOpts().ObjC)
     return true;
----------------
shafik wrote:
> It is not obvious to me if this is a drive by fix or this has a larger effect in the context of this change. Is there a test that demonstrates the effect of this change?
That's a left over from when the patched allowed placeholder in parameter names (EWG voted against that). thanks!


================
Comment at: clang/lib/Sema/SemaDecl.cpp:2024
   if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
+    if (D->isPlaceholderVar())
+      return !VD->hasInit();
----------------
erichkeane wrote:
> Why would we get here?  Doesn't line 1995 pick this up?  Or am I missing where D is modified?
> 
> ALSO, I'd suggest making this VD->isPlaceholderVar, as to not annoy the SA tools.
Yep, this was dead code, thanks!


================
Comment at: clang/lib/Sema/SemaDecl.cpp:18217
 
+  NewFD->setIsPlaceholderVar(LangOpts.CPlusPlus && II && II->isPlaceholder());
   if (PrevDecl && !isa<TagDecl>(PrevDecl)) {
----------------
shafik wrote:
> Why can't we fold this into `FieldDecl::Create`? This comment applies in some other places as well.
By adding a parameter to FieldDecl::Create? We could, I'm not sure it would necessarily be cleaner. Let me know what you prefer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536