[clang] 76f888d - Fix handling of destructor names that name typedefs.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 10 02:21:24 PST 2020


Author: Richard Smith
Date: 2020-02-10T02:21:01-08:00
New Revision: 76f888d0a5324f4c6ae89cac61077cca4299b159

URL: https://github.com/llvm/llvm-project/commit/76f888d0a5324f4c6ae89cac61077cca4299b159
DIFF: https://github.com/llvm/llvm-project/commit/76f888d0a5324f4c6ae89cac61077cca4299b159.diff

LOG: Fix handling of destructor names that name typedefs.

1) Fix a regression in llvmorg-11-init-2485-g0e3a4877840 that would
reject some cases where a class name is shadowed by a typedef-name
causing a destructor declaration to be rejected. Prefer a tag type over
a typedef in destructor name lookup.

2) Convert the "type in destructor declaration is a typedef" error to an
error-by-default ExtWarn to allow codebases to turn it off. GCC and MSVC
do not enforce this rule.

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/include/clang/Sema/Sema.h
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/lib/Sema/SemaExprCXX.cpp
    clang/lib/Sema/SemaLookup.cpp
    clang/test/SemaCXX/destructor.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 82861f0d5d72..5cfd32c57c61 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1909,8 +1909,9 @@ def err_destructor_return_type : Error<"destructor cannot have a return type">;
 def err_destructor_redeclared : Error<"destructor cannot be redeclared">;
 def err_destructor_with_params : Error<"destructor cannot have any parameters">;
 def err_destructor_variadic : Error<"destructor cannot be variadic">;
-def err_destructor_typedef_name : Error<
-  "destructor cannot be declared using a %select{typedef|type alias}1 %0 of the class name">;
+def ext_destructor_typedef_name : ExtWarn<
+  "destructor cannot be declared using a %select{typedef|type alias}1 %0 "
+  "of the class name">, DefaultError, InGroup<DiagGroup<"dtor-typedef">>;
 def err_undeclared_destructor_name : Error<
   "undeclared identifier %0 in destructor name">;
 def err_destructor_name : Error<

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 99cfd2411ab7..3e6856048725 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3479,6 +3479,9 @@ class Sema final {
     /// operator overloading. This lookup is similar to ordinary name
     /// lookup, but will ignore any declarations that are class members.
     LookupOperatorName,
+    /// Look up a name following ~ in a destructor name. This is an ordinary
+    /// lookup, but prefers tags to typedefs.
+    LookupDestructorName,
     /// Look up of a name that precedes the '::' scope resolution
     /// operator in C++. This lookup completely ignores operator, object,
     /// function, and enumerator names (C++ [basic.lookup.qual]p1).

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index a1f7806877c5..7331c3369744 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -9986,12 +9986,12 @@ QualType Sema::CheckDestructorDeclarator(Declarator &D, QualType R,
   //   declaration.
   QualType DeclaratorType = GetTypeFromParser(D.getName().DestructorName);
   if (const TypedefType *TT = DeclaratorType->getAs<TypedefType>())
-    Diag(D.getIdentifierLoc(), diag::err_destructor_typedef_name)
+    Diag(D.getIdentifierLoc(), diag::ext_destructor_typedef_name)
       << DeclaratorType << isa<TypeAliasDecl>(TT->getDecl());
   else if (const TemplateSpecializationType *TST =
              DeclaratorType->getAs<TemplateSpecializationType>())
     if (TST->isTypeAlias())
-      Diag(D.getIdentifierLoc(), diag::err_destructor_typedef_name)
+      Diag(D.getIdentifierLoc(), diag::ext_destructor_typedef_name)
         << DeclaratorType << 1;
 
   // C++ [class.dtor]p2:

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index a39b0b1f7766..83333857234c 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -201,11 +201,9 @@ ParsedType Sema::getDestructorName(SourceLocation TildeLoc,
 
     if (TypeDecl *Type = Found.getAsSingle<TypeDecl>()) {
       QualType T = Context.getTypeDeclType(Type);
-      MarkAnyDeclReferenced(Type->getLocation(), Type, /*OdrUse=*/false);
-
       if (SearchType.isNull() || SearchType->isDependentType() ||
           Context.hasSameUnqualifiedType(T, SearchType)) {
-        // We found our type!
+        MarkAnyDeclReferenced(Type->getLocation(), Type, /*OdrUse=*/false);
         return CreateParsedType(T,
                                 Context.getTrivialTypeSourceInfo(T, NameLoc));
       }
@@ -222,7 +220,7 @@ ParsedType Sema::getDestructorName(SourceLocation TildeLoc,
 
     IsDependent |= SearchType->isDependentType();
 
-    LookupResult Found(*this, &II, NameLoc, LookupOrdinaryName);
+    LookupResult Found(*this, &II, NameLoc, LookupDestructorName);
     DeclContext *LookupCtx = computeDeclContext(SearchType);
     if (!LookupCtx)
       return nullptr;
@@ -239,7 +237,7 @@ ParsedType Sema::getDestructorName(SourceLocation TildeLoc,
     if (!LookupCtx)
       return nullptr;
 
-    LookupResult Found(*this, &II, NameLoc, LookupOrdinaryName);
+    LookupResult Found(*this, &II, NameLoc, LookupDestructorName);
     if (RequireCompleteDeclContext(LookupSS, LookupCtx)) {
       Failed = true;
       return nullptr;
@@ -252,7 +250,7 @@ ParsedType Sema::getDestructorName(SourceLocation TildeLoc,
     if (Failed || !S)
       return nullptr;
 
-    LookupResult Found(*this, &II, NameLoc, LookupOrdinaryName);
+    LookupResult Found(*this, &II, NameLoc, LookupDestructorName);
     LookupName(Found, S);
     return CheckLookupResult(Found);
   };

diff  --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 8d96404a5c27..82a197196576 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -215,6 +215,7 @@ static inline unsigned getIDNS(Sema::LookupNameKind NameKind,
   case Sema::LookupOrdinaryName:
   case Sema::LookupRedeclarationWithLinkage:
   case Sema::LookupLocalFriendName:
+  case Sema::LookupDestructorName:
     IDNS = Decl::IDNS_Ordinary;
     if (CPlusPlus) {
       IDNS |= Decl::IDNS_Tag | Decl::IDNS_Member | Decl::IDNS_Namespace;
@@ -378,11 +379,14 @@ static bool isPreferredLookupResult(Sema &S, Sema::LookupNameKind Kind,
   // type), per a generous reading of C++ [dcl.typedef]p3 and p4. The typedef
   // might carry additional semantic information, such as an alignment override.
   // However, per C++ [dcl.typedef]p5, when looking up a tag name, prefer a tag
-  // declaration over a typedef.
+  // declaration over a typedef. Also prefer a tag over a typedef for
+  // destructor name lookup because in some contexts we only accept a
+  // class-name in a destructor declaration.
   if (DUnderlying->getCanonicalDecl() != EUnderlying->getCanonicalDecl()) {
     assert(isa<TypeDecl>(DUnderlying) && isa<TypeDecl>(EUnderlying));
     bool HaveTag = isa<TagDecl>(EUnderlying);
-    bool WantTag = Kind == Sema::LookupTagName;
+    bool WantTag =
+        Kind == Sema::LookupTagName || Kind == Sema::LookupDestructorName;
     return HaveTag != WantTag;
   }
 
@@ -2297,6 +2301,7 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
     case LookupMemberName:
     case LookupRedeclarationWithLinkage:
     case LookupLocalFriendName:
+    case LookupDestructorName:
       BaseCallback = &CXXRecordDecl::FindOrdinaryMember;
       break;
 

diff  --git a/clang/test/SemaCXX/destructor.cpp b/clang/test/SemaCXX/destructor.cpp
index 92afc1256ced..7fe1cac3c7e7 100644
--- a/clang/test/SemaCXX/destructor.cpp
+++ b/clang/test/SemaCXX/destructor.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -fsyntax-only -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -fcxx-exceptions -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple %ms_abi_triple -DMSABI -fsyntax-only -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -fsyntax-only -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -fcxx-exceptions -verify %s -pedantic
+// RUN: %clang_cc1 -std=c++11 -triple %ms_abi_triple -DMSABI -fsyntax-only -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -verify %s -pedantic
 
 #if defined(BE_THE_HEADER)
 
@@ -492,4 +492,22 @@ void foo1() {
   x.foo1();
 }
 }
+
+namespace DtorTypedef {
+  struct A { ~A(); };
+  using A = A;
+  DtorTypedef::A::~A() {}
+
+  // This is invalid, but compilers accept it.
+  struct B { ~B(); };
+  namespace N { using B = B; }
+  N::B::~B() {} // expected-error {{destructor cannot be declared using a type alias}}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdtor-typedef"
+  struct C { ~C(); };
+  namespace N { using C = C; }
+  N::C::~C() {}
+#pragma clang diagnostic pop
+}
 #endif // BE_THE_HEADER


        


More information about the cfe-commits mailing list