erichkeane 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;
----------------
rZhBoYao wrote:
> 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.
I'm not sure what the side effect of that is... Can you give it a shot?
================
Comment at: clang/test/SemaCXX/PR28101.cpp:8
+
+#ifdef CASE_1
+
----------------
rZhBoYao wrote:
> 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.
They shouldn't crash afterwards, right? So this shouldn't be a problem.
================
Comment at: clang/test/SemaCXX/PR28101.cpp:16
+
+A<int> instantiate() { return {nullptr}; }
+
----------------
rZhBoYao wrote:
> 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.
I think we definitely need to do so there, the instantiation trace not being
here makes this a much less useful diagnostic.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115248/new/
https://reviews.llvm.org/D115248
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
