Related to PR37768: improve diagnostics for class name shadowing.

Diagnose the name of the class being shadowed by using declarations, and
improve the diagnostics for the case where the name of the class is
shadowed by a non-static data member in a class with constructors.  In
the latter case, we now always give the "member with the same name as
its class" diagnostic regardless of the relative order of the member and
the constructor, rather than giving an inscrutible diagnostic if the
constructor appears second.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@335182 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 76418143199cbc567ef6959d73f3ac581218f9e2..8b4cf02fdeb7f9f5a615909bac6ed0843d5b2597 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -4984,6 +4984,8 @@ public:
                                           SourceLocation NameLoc,
                                           IdentifierInfo &Name);
 
+  ParsedType getConstructorName(IdentifierInfo &II, SourceLocation NameLoc,
+                                Scope *S, CXXScopeSpec &SS);
   ParsedType getDestructorName(SourceLocation TildeLoc,
                                IdentifierInfo &II, SourceLocation NameLoc,
                                Scope *S, CXXScopeSpec &SS,
@@ -5704,6 +5706,7 @@ public:
   //===--------------------------------------------------------------------===//
   // C++ Classes
   //
+  CXXRecordDecl *getCurrentClass(Scope *S, const CXXScopeSpec *SS);
   bool isCurrentClassName(const IdentifierInfo &II, Scope *S,
                           const CXXScopeSpec *SS = nullptr);
   bool isCurrentClassNameTypo(IdentifierInfo *&II, const CXXScopeSpec *SS);

diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 83bfd46d8931457bfd500e42b82a3ace86c0ef20..e3691565480b93afeb0cdeba754edb83d1a39012 100644 (file)
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -3250,6 +3250,13 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
         continue;
       }
 
+      // If we're in a context where the identifier could be a class name,
+      // check whether this is a constructor declaration.
+      if (getLangOpts().CPlusPlus && DSContext == DeclSpecContext::DSC_class &&
+          Actions.isCurrentClassName(*Tok.getIdentifierInfo(), getCurScope()) &&
+          isConstructorDeclarator(/*Unqualified*/true))
+        goto DoneWithDeclSpec;
+
       ParsedType TypeRep = Actions.getTypeName(
           *Tok.getIdentifierInfo(), Tok.getLocation(), getCurScope(), nullptr,
           false, false, nullptr, false, false,
@@ -3269,13 +3276,6 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
         goto DoneWithDeclSpec;
       }
 
-      // If we're in a context where the identifier could be a class name,
-      // check whether this is a constructor declaration.
-      if (getLangOpts().CPlusPlus && DSContext == DeclSpecContext::DSC_class &&
-          Actions.isCurrentClassName(*Tok.getIdentifierInfo(), getCurScope()) &&
-          isConstructorDeclarator(/*Unqualified*/true))
-        goto DoneWithDeclSpec;
-
       // Likewise, if this is a context where the identifier could be a template
       // name, check whether this is a deduction guide declaration.
       if (getLangOpts().CPlusPlus17 &&

diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp
index ad91ff00210b510b5c360c17a4155eb319c6b10e..26b8a0f7793fca6945cd65791509e9e2a77e41f9 100644 (file)
--- a/lib/Parse/ParseExprCXX.cpp
+++ b/lib/Parse/ParseExprCXX.cpp
@@ -2505,10 +2505,9 @@ bool Parser::ParseUnqualifiedId(CXXScopeSpec &SS, bool EnteringContext,
     if (AllowConstructorName && 
         Actions.isCurrentClassName(*Id, getCurScope(), &SS)) {
       // We have parsed a constructor name.
-      ParsedType Ty = Actions.getTypeName(*Id, IdLoc, getCurScope(), &SS, false,
-                                          false, nullptr,
-                                          /*IsCtorOrDtorName=*/true,
-                                          /*NonTrivialTypeSourceInfo=*/true);
+      ParsedType Ty = Actions.getConstructorName(*Id, IdLoc, getCurScope(), SS);
+      if (!Ty)
+        return true;
       Result.setConstructorName(Ty, IdLoc, IdLoc);
     } else if (getLangOpts().CPlusPlus17 &&
                AllowDeductionGuide && SS.isEmpty() &&
@@ -2555,11 +2554,10 @@ bool Parser::ParseUnqualifiedId(CXXScopeSpec &SS, bool EnteringContext,
           << TemplateId->Name
           << FixItHint::CreateRemoval(
                     SourceRange(TemplateId->LAngleLoc, TemplateId->RAngleLoc));
-        ParsedType Ty =
-            Actions.getTypeName(*TemplateId->Name, TemplateId->TemplateNameLoc,
-                                getCurScope(), &SS, false, false, nullptr,
-                                /*IsCtorOrDtorName=*/true,
-                                /*NontrivialTypeSourceInfo=*/true);
+        ParsedType Ty = Actions.getConstructorName(
+            *TemplateId->Name, TemplateId->TemplateNameLoc, getCurScope(), SS);
+        if (!Ty)
+          return true;
         Result.setConstructorName(Ty, TemplateId->TemplateNameLoc,
                                   TemplateId->RAngleLoc);
         ConsumeAnnotationToken();

diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 7a7e0378cfdca69085f9e50c4be279d91b930272..f397949964b7bdf5af487992a522da961a3f4df2 100644 (file)
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -2059,24 +2059,36 @@ bool Sema::CheckConstexprFunctionBody(const FunctionDecl *Dcl, Stmt *Body) {
   return true;
 }
 
+/// Get the class that is directly named by the current context. This is the
+/// class for which an unqualified-id in this scope could name a constructor
+/// or destructor.
+///
+/// If the scope specifier denotes a class, this will be that class.
+/// If the scope specifier is empty, this will be the class whose
+/// member-specification we are currently within. Otherwise, there
+/// is no such class.
+CXXRecordDecl *Sema::getCurrentClass(Scope *, const CXXScopeSpec *SS) {
+  assert(getLangOpts().CPlusPlus && "No class names in C!");
+
+  if (SS && SS->isInvalid())
+    return nullptr;
+
+  if (SS && SS->isNotEmpty()) {
+    DeclContext *DC = computeDeclContext(*SS, true);
+    return dyn_cast_or_null<CXXRecordDecl>(DC);
+  }
+
+  return dyn_cast_or_null<CXXRecordDecl>(CurContext);
+}
+
 /// isCurrentClassName - Determine whether the identifier II is the
 /// name of the class type currently being defined. In the case of
 /// nested classes, this will only return true if II is the name of
 /// the innermost class.
-bool Sema::isCurrentClassName(const IdentifierInfo &II, Scope *,
+bool Sema::isCurrentClassName(const IdentifierInfo &II, Scope *S,
                               const CXXScopeSpec *SS) {
-  assert(getLangOpts().CPlusPlus && "No class names in C!");
-
-  CXXRecordDecl *CurDecl;
-  if (SS && SS->isSet() && !SS->isInvalid()) {
-    DeclContext *DC = computeDeclContext(*SS, true);
-    CurDecl = dyn_cast_or_null<CXXRecordDecl>(DC);
-  } else
-    CurDecl = dyn_cast_or_null<CXXRecordDecl>(CurContext);
-
-  if (CurDecl && CurDecl->getIdentifier())
-    return &II == CurDecl->getIdentifier();
-  return false;
+  CXXRecordDecl *CurDecl = getCurrentClass(S, SS);
+  return CurDecl && &II == CurDecl->getIdentifier();
 }
 
 /// Determine whether the identifier II is a typo for the name of
@@ -5991,10 +6003,11 @@ void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) {
     DeclContext::lookup_result R = Record->lookup(Record->getDeclName());
     for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E;
          ++I) {
-      NamedDecl *D = *I;
-      if ((isa<FieldDecl>(D) && Record->hasUserDeclaredConstructor()) ||
+      NamedDecl *D = (*I)->getUnderlyingDecl();
+      if (((isa<FieldDecl>(D) || isa<UnresolvedUsingValueDecl>(D)) &&
+           Record->hasUserDeclaredConstructor()) ||
           isa<IndirectFieldDecl>(D)) {
-        Diag(D->getLocation(), diag::err_member_name_of_class)
+        Diag((*I)->getLocation(), diag::err_member_name_of_class)
           << D->getDeclName();
         break;
       }
@@ -9538,6 +9551,19 @@ bool Sema::CheckUsingShadowDecl(UsingDecl *Using, NamedDecl *Orig,
     if (isa<UsingDecl>(D) || isa<UsingPackDecl>(D))
       continue;
 
+    if (auto *RD = dyn_cast<CXXRecordDecl>(D)) {
+      // C++ [class.mem]p19:
+      //   If T is the name of a class, then [every named member other than
+      //   a non-static data member] shall have a name different from T
+      if (RD->isInjectedClassName() && !isa<FieldDecl>(Target) &&
+          !isa<IndirectFieldDecl>(Target) &&
+          !isa<UnresolvedUsingValueDecl>(Target) &&
+          DiagnoseClassNameShadow(
+              CurContext,
+              DeclarationNameInfo(Using->getDeclName(), Using->getLocation())))
+        return true;
+    }
+
     if (IsEquivalentForUsingDecl(Context, D, Target)) {
       if (UsingShadowDecl *Shadow = dyn_cast<UsingShadowDecl>(*I))
         PrevShadow = Shadow;

diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 5d1001dc84d9519648d21fbbdbe068d4ad865fff..d977ea3453885158f977812ed89aef8596ac54d2 100644 (file)
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -80,6 +80,39 @@ ParsedType Sema::getInheritingConstructorName(CXXScopeSpec &SS,
                           Context.getTrivialTypeSourceInfo(Type, NameLoc));
 }
 
+ParsedType Sema::getConstructorName(IdentifierInfo &II,
+                                    SourceLocation NameLoc,
+                                    Scope *S, CXXScopeSpec &SS) {
+  CXXRecordDecl *CurClass = getCurrentClass(S, &SS);
+  assert(CurClass && &II == CurClass->getIdentifier() &&
+         "not a constructor name");
+
+  if (SS.isNotEmpty() && RequireCompleteDeclContext(SS, CurClass))
+    return ParsedType();
+
+  // Find the injected-class-name declaration. Note that we make no attempt to
+  // diagnose cases where the injected-class-name is shadowed: the only
+  // declaration that can validly shadow the injected-class-name is a
+  // non-static data member, and if the class contains both a non-static data
+  // member and a constructor then it is ill-formed (we check that in
+  // CheckCompletedCXXClass).
+  CXXRecordDecl *InjectedClassName = nullptr;
+  for (NamedDecl *ND : CurClass->lookup(&II)) {
+    auto *RD = dyn_cast<CXXRecordDecl>(ND);
+    if (RD && RD->isInjectedClassName()) {
+      InjectedClassName = RD;
+      break;
+    }
+  }
+  assert(InjectedClassName && "couldn't find injected class name");
+
+  QualType T = Context.getTypeDeclType(InjectedClassName);
+  DiagnoseUseOfDecl(InjectedClassName, NameLoc);
+  MarkAnyDeclReferenced(NameLoc, InjectedClassName, /*OdrUse=*/false);
+
+  return ParsedType::make(T);
+}
+
 ParsedType Sema::getDestructorName(SourceLocation TildeLoc,
                                    IdentifierInfo &II,
                                    SourceLocation NameLoc,

diff --git a/test/CXX/class/class.mem/p13.cpp b/test/CXX/class/class.mem/p13.cpp
index bc01fd4d30c5698886ddafb52a5be2b978fb86c3..1b1c0c7f8fc32230b9edd799a74b7a2ded438e60 100644 (file)
--- a/test/CXX/class/class.mem/p13.cpp
+++ b/test/CXX/class/class.mem/p13.cpp
@@ -67,3 +67,50 @@ struct X4 { // expected-note{{previous}}
     };
   };
 };
+
+// This includes such things inherited from base classes.
+struct B {
+  static int D0;
+  int Da() {};
+  enum D1 {};
+  struct D1a;
+  typedef int D2;
+  using D2a = int;
+  template<typename T> struct D2b;
+  template<typename T> void D2c();
+  template<typename T> static int D2d;
+  template<typename T> using D2e = int;
+  union { int D4; };
+  int Dtemplate;
+  int Dtemplate_with_ctors;
+};
+struct B2 { int Dtemplate(); };
+
+struct D0 : B { using B::D0; }; // expected-error {{member 'D0' has the same name as its class}}
+struct Da : B { using B::Da; }; // expected-error {{member 'Da' has the same name as its class}}
+struct D1 : B { using B::D1; }; // expected-error {{member 'D1' has the same name as its class}}
+struct D1a : B { using B::D1a; }; // expected-error {{member 'D1a' has the same name as its class}}
+struct D2 : B { using B::D2; }; // expected-error {{member 'D2' has the same name as its class}}
+struct D2a : B { using B::D2a; }; // expected-error {{member 'D2a' has the same name as its class}}
+struct D2b : B { using B::D2b; }; // expected-error {{member 'D2b' has the same name as its class}}
+struct D2c : B { using B::D2c; }; // expected-error {{member 'D2c' has the same name as its class}}
+struct D2d : B { using B::D2d; }; // expected-error {{member 'D2d' has the same name as its class}}
+struct D2e : B { using B::D2e; }; // expected-error {{member 'D2e' has the same name as its class}}
+struct D4 : B { using B::D4; }; // expected-error {{member 'D4' has the same name as its class}}
+
+template<typename B> struct Dtemplate : B {
+  using B::Dtemplate; // expected-error {{member 'Dtemplate' has the same name as its class}}
+};
+Dtemplate<B> ok;
+Dtemplate<B2> error; // expected-note {{in instantiation of}}
+
+template<typename B> struct Dtemplate_with_ctors : B {
+  Dtemplate_with_ctors();
+  using B::Dtemplate_with_ctors; // expected-error {{member 'Dtemplate_with_ctors' has the same name as its class}}
+};
+
+template<typename B> struct CtorDtorName : B {
+  using B::CtorDtorName; // expected-error {{member 'CtorDtorName' has the same name as its class}}
+  CtorDtorName();
+  ~CtorDtorName(); // expected-error {{expected the class name after '~' to name a destructor}}
+};

diff --git a/test/CXX/class/class.mem/p14.cpp b/test/CXX/class/class.mem/p14.cpp
index 3f14099ef89612a59ef27ea4d4ed0c95002f86e9..a2f60618565d0fef339278f0c171624e087fcf13 100644 (file)
--- a/test/CXX/class/class.mem/p14.cpp
+++ b/test/CXX/class/class.mem/p14.cpp
@@ -9,9 +9,8 @@ struct X0 {
 };
 
 struct X1 {
-  int X1; // expected-note{{hidden by a non-type declaration of 'X1' here}}
-  X1(); // expected-error{{must use 'struct' tag to refer to type 'X1' in this scope}} \
-        // expected-error{{expected member name or ';' after declaration specifiers}}
+  int X1; // expected-error{{member 'X1' has the same name as its class}}
+  X1();
 };
 
 struct X2 {

diff --git a/test/CXX/drs/dr0xx.cpp b/test/CXX/drs/dr0xx.cpp
index a311d8f339327de64f913860f252cca9a5e85f69..048187814db6a8f33a50d027ededf0d7ba73c457 100644 (file)
--- a/test/CXX/drs/dr0xx.cpp
+++ b/test/CXX/drs/dr0xx.cpp
@@ -911,9 +911,8 @@ namespace dr80 { // dr80: yes
     static int B; // expected-error {{same name as its class}}
   };
   struct C {
-    int C; // expected-note {{hidden by}}
-    // FIXME: These diagnostics aren't very good.
-    C(); // expected-error {{must use 'struct' tag to refer to}} expected-error {{expected member name}}
+    int C; // expected-error {{same name as its class}}
+    C();
   };
   struct D {
     D();