[PATCH] D115248: [Clang] Fix PR28101

[Erich Keane via Phabricator via cfe-commits]

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


================
Comment at: clang/test/SemaCXX/PR28101.cpp:8
+
+#ifdef CASE_1
+
----------------
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.


================
Comment at: clang/test/SemaCXX/PR28101.cpp:13
+  T(A<T>){}; // expected-error{{member 'A' cannot have template arguments}}\
+  // expected-error{{member 'A' has the same name as its class}}
+};
----------------
This second error here is strange, I don't see it as particularly useful here.

I see gcc at least gives the full name (int `A<int>::A`), perhaps that makes this more useful?


================
Comment at: clang/test/SemaCXX/PR28101.cpp:16
+
+A<int> instantiate() { return {nullptr}; }
+
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115248