[PATCH] D115248: [Clang] Fix PR28101

[PoYao Chang via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:3430
           << D.getName().TemplateId->LAngleLoc;
+      if (cast<CXXRecordDecl>(CurContext)->getDeclName() == Name)
+        Diag(Loc, diag::err_member_name_of_class) << Name;
----------------
erichkeane wrote:
> I see we are diagnosing this ONLY inside of the case where it is a template.  Where is this caught when we are in the non-template case, why doesn't THAT catch these cases, and what would it take to do so?
CheckCompletedCXXClass should catch diag::err_member_name_of_class. However, these FieldDecl have no name hence uncaught. Maybe we should set the identifier for this declarator here.


================
Comment at: clang/test/SemaCXX/PR28101.cpp:8
+
+#ifdef CASE_1
+
----------------
erichkeane wrote:
> This isn't necessary, the compiler in verify mode should see all errors.  I see above at most 2 invocations of the compiler as necessary.
Instantiation of each of the 4 classes could crash on its own, so I thought I'd separate them into 4 invocations.


================
Comment at: clang/test/SemaCXX/PR28101.cpp:16
+
+A<int> instantiate() { return {nullptr}; }
+
----------------
erichkeane wrote:
> How come there are no notes in this test that say 'in instantiation of...'?  I would expect those in at least some of these cases, right?
Because it was caught before instantiation and when instantiating the FieldDecl is gone after calling `D.setInvalidType();`. If I set the identifier for the FieldDecl and let `CheckCompletedCXXClass` handle the "has the same name as its class" error, there will be the "in instantiation of..." note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115248