[clang] 0e3a487 - PR12350: Handle remaining cases permitted by CWG DR 244.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 7 18:40:56 PST 2020


Author: Richard Smith
Date: 2020-02-07T18:40:41-08:00
New Revision: 0e3a48778408b505946e465abf5c77a2ddd4918c

URL: https://github.com/llvm/llvm-project/commit/0e3a48778408b505946e465abf5c77a2ddd4918c
DIFF: https://github.com/llvm/llvm-project/commit/0e3a48778408b505946e465abf5c77a2ddd4918c.diff

LOG: PR12350: Handle remaining cases permitted by CWG DR 244.

Also add extension warnings for the cases that are disallowed by the
current rules for destructor name lookup, refactor and simplify the
lookup code, and improve the diagnostic quality when lookup fails.

The special case we previously supported for converting
p->N::S<int>::~S() from naming a class template into naming a
specialization thereof is subsumed by a more general rule here (which is
also consistent with Clang's historical behavior and that of other
compilers): if we can't find a suitable S in N, also look in N::S<int>.

The extension warnings are off by default, except for a warning when
lookup for p->N::S::~T() looks for T in scope instead of in N (or N::S).
That seems sufficiently heinous to warn on by default, especially since
we can't support it for a dependent nested-name-specifier.

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticGroups.td
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/AST/NestedNameSpecifier.cpp
    clang/lib/Sema/DeclSpec.cpp
    clang/lib/Sema/SemaExprCXX.cpp
    clang/test/CXX/class/class.mem/p13.cpp
    clang/test/CXX/drs/dr2xx.cpp
    clang/test/CXX/drs/dr3xx.cpp
    clang/test/FixIt/fixit.cpp
    clang/test/Parser/cxx-decl.cpp
    clang/test/SemaCXX/constructor.cpp
    clang/test/SemaCXX/destructor.cpp
    clang/test/SemaCXX/pseudo-destructors.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index a2bc29986a07..8c54723cdbab 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -192,6 +192,7 @@ def CXX2aDesignator : DiagGroup<"c++2a-designator">;
 // designators (including the warning controlled by -Wc++2a-designator).
 def C99Designator : DiagGroup<"c99-designator", [CXX2aDesignator]>;
 def GNUDesignator : DiagGroup<"gnu-designator">;
+def DtorName : DiagGroup<"dtor-name">;
 
 def DynamicExceptionSpec
     : DiagGroup<"dynamic-exception-spec", [DeprecatedDynamicExceptionSpec]>;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9de60d3a8d27..82861f0d5d72 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1911,17 +1911,33 @@ 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 err_undeclared_destructor_name : Error<
+  "undeclared identifier %0 in destructor name">;
 def err_destructor_name : Error<
   "expected the class name after '~' to name the enclosing class">;
-def err_destructor_class_name : Error<
-  "expected the class name after '~' to name a destructor">;
-def err_ident_in_dtor_not_a_type : Error<
+def err_destructor_name_nontype : Error<
+  "identifier %0 after '~' in destructor name does not name a type">;
+def err_destructor_expr_mismatch : Error<
+  "identifier %0 in object destruction expression does not name the type "
+  "%1 of the object being destroyed">;
+def err_destructor_expr_nontype : Error<
   "identifier %0 in object destruction expression does not name a type">;
 def err_destructor_expr_type_mismatch : Error<
   "destructor type %0 in object destruction expression does not match the "
   "type %1 of the object being destroyed">;
 def note_destructor_type_here : Note<
-  "type %0 is declared here">;
+  "type %0 found by destructor name lookup">;
+def note_destructor_nontype_here : Note<
+  "non-type declaration found by destructor name lookup">;
+def ext_dtor_named_in_wrong_scope : Extension<
+  "ISO C++ requires the name after '::~' to be found in the same scope as "
+  "the name before '::~'">, InGroup<DtorName>;
+def ext_dtor_name_missing_template_arguments : Extension<
+  "ISO C++ requires template argument list in destructor name">,
+  InGroup<DtorName>;
+def ext_qualified_dtor_named_in_lexical_scope : ExtWarn<
+  "qualified destructor name only found in lexical scope; omit the qualifier "
+  "to find this type name by unqualified lookup">, InGroup<DtorName>;
 
 def err_destroy_attr_on_non_static_var : Error<
   "%select{no_destroy|always_destroy}0 attribute can only be applied to a"

diff  --git a/clang/lib/AST/NestedNameSpecifier.cpp b/clang/lib/AST/NestedNameSpecifier.cpp
index 137953fa8203..81130512bfe1 100644
--- a/clang/lib/AST/NestedNameSpecifier.cpp
+++ b/clang/lib/AST/NestedNameSpecifier.cpp
@@ -482,10 +482,9 @@ static void Append(char *Start, char *End, char *&Buffer, unsigned &BufferSize,
         (unsigned)(BufferCapacity ? BufferCapacity * 2 : sizeof(void *) * 2),
         (unsigned)(BufferSize + (End - Start)));
     char *NewBuffer = static_cast<char *>(llvm::safe_malloc(NewCapacity));
-    if (BufferCapacity) {
-      memcpy(NewBuffer, Buffer, BufferSize);
+    memcpy(NewBuffer, Buffer, BufferSize);
+    if (BufferCapacity)
       free(Buffer);
-    }
     Buffer = NewBuffer;
     BufferCapacity = NewCapacity;
   }

diff  --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index 94d87974624e..eca97734bc9d 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -130,6 +130,8 @@ void CXXScopeSpec::Adopt(NestedNameSpecifierLoc Other) {
 
   Range = Other.getSourceRange();
   Builder.Adopt(Other);
+  assert(Range == Builder.getSourceRange() &&
+         "NestedNameSpecifierLoc range computation incorrect");
 }
 
 SourceLocation CXXScopeSpec::getLastQualifierNameLoc() const {

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index e53281d11755..a39b0b1f7766 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -156,103 +156,48 @@ ParsedType Sema::getDestructorName(SourceLocation TildeLoc,
   //   }
   //
   // See also PR6358 and PR6359.
-  // For this reason, we're currently only doing the C++03 version of this
-  // code; the C++0x version has to wait until we get a proper spec.
-  QualType SearchType;
-  DeclContext *LookupCtx = nullptr;
-  bool isDependent = false;
-  bool LookInScope = false;
+  //
+  // For now, we accept all the cases in which the name given could plausibly
+  // be interpreted as a correct destructor name, issuing off-by-default
+  // extension diagnostics on the cases that don't strictly conform to the
+  // C++20 rules. This basically means we always consider looking in the
+  // nested-name-specifier prefix, the complete nested-name-specifier, and
+  // the scope, and accept if we find the expected type in any of the three
+  // places.
 
   if (SS.isInvalid())
     return nullptr;
 
+  // Whether we've failed with a diagnostic already.
+  bool Failed = false;
+
+  llvm::SmallVector<NamedDecl*, 8> FoundDecls;
+  llvm::SmallSet<CanonicalDeclPtr<Decl>, 8> FoundDeclSet;
+
   // If we have an object type, it's because we are in a
   // pseudo-destructor-expression or a member access expression, and
   // we know what type we're looking for.
-  if (ObjectTypePtr)
-    SearchType = GetTypeFromParser(ObjectTypePtr);
-
-  if (SS.isSet()) {
-    NestedNameSpecifier *NNS = SS.getScopeRep();
-
-    bool AlreadySearched = false;
-    bool LookAtPrefix = true;
-    // C++11 [basic.lookup.qual]p6:
-    //   If a pseudo-destructor-name (5.2.4) contains a nested-name-specifier,
-    //   the type-names are looked up as types in the scope designated by the
-    //   nested-name-specifier. Similarly, in a qualified-id of the form:
-    //
-    //     nested-name-specifier[opt] class-name :: ~ class-name
-    //
-    //   the second class-name is looked up in the same scope as the first.
-    //
-    // Here, we determine whether the code below is permitted to look at the
-    // prefix of the nested-name-specifier.
-    DeclContext *DC = computeDeclContext(SS, EnteringContext);
-    if (DC && DC->isFileContext()) {
-      AlreadySearched = true;
-      LookupCtx = DC;
-      isDependent = false;
-    } else if (DC && isa<CXXRecordDecl>(DC)) {
-      LookAtPrefix = false;
-      LookInScope = true;
-    }
-
-    // The second case from the C++03 rules quoted further above.
-    NestedNameSpecifier *Prefix = nullptr;
-    if (AlreadySearched) {
-      // Nothing left to do.
-    } else if (LookAtPrefix && (Prefix = NNS->getPrefix())) {
-      CXXScopeSpec PrefixSS;
-      PrefixSS.Adopt(NestedNameSpecifierLoc(Prefix, SS.location_data()));
-      LookupCtx = computeDeclContext(PrefixSS, EnteringContext);
-      isDependent = isDependentScopeSpecifier(PrefixSS);
-    } else if (ObjectTypePtr) {
-      LookupCtx = computeDeclContext(SearchType);
-      isDependent = SearchType->isDependentType();
-    } else {
-      LookupCtx = computeDeclContext(SS, EnteringContext);
-      isDependent = LookupCtx && LookupCtx->isDependentContext();
-    }
-  } else if (ObjectTypePtr) {
-    // C++ [basic.lookup.classref]p3:
-    //   If the unqualified-id is ~type-name, the type-name is looked up
-    //   in the context of the entire postfix-expression. If the type T
-    //   of the object expression is of a class type C, the type-name is
-    //   also looked up in the scope of class C. At least one of the
-    //   lookups shall find a name that refers to (possibly
-    //   cv-qualified) T.
-    LookupCtx = computeDeclContext(SearchType);
-    isDependent = SearchType->isDependentType();
-    assert((isDependent || !SearchType->isIncompleteType()) &&
-           "Caller should have completed object type");
-
-    LookInScope = true;
-  } else {
-    // Perform lookup into the current scope (only).
-    LookInScope = true;
-  }
-
-  TypeDecl *NonMatchingTypeDecl = nullptr;
-  LookupResult Found(*this, &II, NameLoc, LookupOrdinaryName);
-  for (unsigned Step = 0; Step != 2; ++Step) {
-    // Look for the name first in the computed lookup context (if we
-    // have one) and, if that fails to find a match, in the scope (if
-    // we're allowed to look there).
-    Found.clear();
-    if (Step == 0 && LookupCtx) {
-      if (RequireCompleteDeclContext(SS, LookupCtx))
-        return nullptr;
-      LookupQualifiedName(Found, LookupCtx);
-    } else if (Step == 1 && LookInScope && S) {
-      LookupName(Found, S);
-    } else {
-      continue;
-    }
+  QualType SearchType =
+      ObjectTypePtr ? GetTypeFromParser(ObjectTypePtr) : QualType();
 
+  auto CheckLookupResult = [&](LookupResult &Found) -> ParsedType {
     // FIXME: Should we be suppressing ambiguities here?
-    if (Found.isAmbiguous())
+    if (Found.isAmbiguous()) {
+      Failed = true;
       return nullptr;
+    }
+
+    for (NamedDecl *D : Found) {
+      // Don't list a class twice in the lookup failure diagnostic if it's
+      // found by both its injected-class-name and by the name in the enclosing
+      // scope.
+      if (auto *RD = dyn_cast<CXXRecordDecl>(D))
+        if (RD->isInjectedClassName())
+          D = cast<NamedDecl>(RD->getParent());
+
+      if (FoundDeclSet.insert(D).second)
+        FoundDecls.push_back(D);
+    }
 
     if (TypeDecl *Type = Found.getAsSingle<TypeDecl>()) {
       QualType T = Context.getTypeDeclType(Type);
@@ -261,91 +206,121 @@ ParsedType Sema::getDestructorName(SourceLocation TildeLoc,
       if (SearchType.isNull() || SearchType->isDependentType() ||
           Context.hasSameUnqualifiedType(T, SearchType)) {
         // We found our type!
-
         return CreateParsedType(T,
                                 Context.getTrivialTypeSourceInfo(T, NameLoc));
       }
+    }
 
-      if (!SearchType.isNull())
-        NonMatchingTypeDecl = Type;
-    }
-
-    // If the name that we found is a class template name, and it is
-    // the same name as the template name in the last part of the
-    // nested-name-specifier (if present) or the object type, then
-    // this is the destructor for that class.
-    // FIXME: This is a workaround until we get real drafting for core
-    // issue 399, for which there isn't even an obvious direction.
-    if (ClassTemplateDecl *Template = Found.getAsSingle<ClassTemplateDecl>()) {
-      QualType MemberOfType;
-      if (SS.isSet()) {
-        if (DeclContext *Ctx = computeDeclContext(SS, EnteringContext)) {
-          // Figure out the type of the context, if it has one.
-          if (CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(Ctx))
-            MemberOfType = Context.getTypeDeclType(Record);
-        }
-      }
-      if (MemberOfType.isNull())
-        MemberOfType = SearchType;
+    return nullptr;
+  };
 
-      if (MemberOfType.isNull())
-        continue;
+  bool IsDependent = false;
 
-      // We're referring into a class template specialization. If the
-      // class template we found is the same as the template being
-      // specialized, we found what we are looking for.
-      if (const RecordType *Record = MemberOfType->getAs<RecordType>()) {
-        if (ClassTemplateSpecializationDecl *Spec
-              = dyn_cast<ClassTemplateSpecializationDecl>(Record->getDecl())) {
-          if (Spec->getSpecializedTemplate()->getCanonicalDecl() ==
-                Template->getCanonicalDecl())
-            return CreateParsedType(
-                MemberOfType,
-                Context.getTrivialTypeSourceInfo(MemberOfType, NameLoc));
-        }
+  auto LookupInObjectType = [&]() -> ParsedType {
+    if (Failed || SearchType.isNull())
+      return nullptr;
 
-        continue;
-      }
+    IsDependent |= SearchType->isDependentType();
 
-      // We're referring to an unresolved class template
-      // specialization. Determine whether we class template we found
-      // is the same as the template being specialized or, if we don't
-      // know which template is being specialized, that it at least
-      // has the same name.
-      if (const TemplateSpecializationType *SpecType
-            = MemberOfType->getAs<TemplateSpecializationType>()) {
-        TemplateName SpecName = SpecType->getTemplateName();
-
-        // The class template we found is the same template being
-        // specialized.
-        if (TemplateDecl *SpecTemplate = SpecName.getAsTemplateDecl()) {
-          if (SpecTemplate->getCanonicalDecl() == Template->getCanonicalDecl())
-            return CreateParsedType(
-                MemberOfType,
-                Context.getTrivialTypeSourceInfo(MemberOfType, NameLoc));
+    LookupResult Found(*this, &II, NameLoc, LookupOrdinaryName);
+    DeclContext *LookupCtx = computeDeclContext(SearchType);
+    if (!LookupCtx)
+      return nullptr;
+    LookupQualifiedName(Found, LookupCtx);
+    return CheckLookupResult(Found);
+  };
 
-          continue;
-        }
+  auto LookupInNestedNameSpec = [&](CXXScopeSpec &LookupSS) -> ParsedType {
+    if (Failed)
+      return nullptr;
 
-        // The class template we found has the same name as the
-        // (dependent) template name being specialized.
-        if (DependentTemplateName *DepTemplate
-                                    = SpecName.getAsDependentTemplateName()) {
-          if (DepTemplate->isIdentifier() &&
-              DepTemplate->getIdentifier() == Template->getIdentifier())
-            return CreateParsedType(
-                MemberOfType,
-                Context.getTrivialTypeSourceInfo(MemberOfType, NameLoc));
+    IsDependent |= isDependentScopeSpecifier(LookupSS);
+    DeclContext *LookupCtx = computeDeclContext(LookupSS, EnteringContext);
+    if (!LookupCtx)
+      return nullptr;
 
-          continue;
-        }
-      }
+    LookupResult Found(*this, &II, NameLoc, LookupOrdinaryName);
+    if (RequireCompleteDeclContext(LookupSS, LookupCtx)) {
+      Failed = true;
+      return nullptr;
     }
+    LookupQualifiedName(Found, LookupCtx);
+    return CheckLookupResult(Found);
+  };
+
+  auto LookupInScope = [&]() -> ParsedType {
+    if (Failed || !S)
+      return nullptr;
+
+    LookupResult Found(*this, &II, NameLoc, LookupOrdinaryName);
+    LookupName(Found, S);
+    return CheckLookupResult(Found);
+  };
+
+  // C++2a [basic.lookup.qual]p6:
+  //   In a qualified-id of the form
+  //
+  //     nested-name-specifier[opt] type-name :: ~ type-name
+  //
+  //   the second type-name is looked up in the same scope as the first.
+  //
+  // We interpret this as meaning that if you do a dual-scope lookup for the
+  // first name, you also do a dual-scope lookup for the second name, per
+  // C++ [basic.lookup.classref]p4:
+  //
+  //   If the id-expression in a class member access is a qualified-id of the
+  //   form
+  //
+  //     class-name-or-namespace-name :: ...
+  //
+  //   the class-name-or-namespace-name following the . or -> is first looked
+  //   up in the class of the object expression and the name, if found, is used.
+  //   Otherwise, it is looked up in the context of the entire
+  //   postfix-expression.
+  //
+  // This looks in the same scopes as for an unqualified destructor name:
+  //
+  // C++ [basic.lookup.classref]p3:
+  //   If the unqualified-id is ~ type-name, the type-name is looked up
+  //   in the context of the entire postfix-expression. If the type T
+  //   of the object expression is of a class type C, the type-name is
+  //   also looked up in the scope of class C. At least one of the
+  //   lookups shall find a name that refers to cv T.
+  //
+  // FIXME: The intent is unclear here. Should type-name::~type-name look in
+  // the scope anyway if it finds a non-matching name declared in the class?
+  // If both lookups succeed and find a dependent result, which result should
+  // we retain? (Same question for p->~type-name().)
+
+  if (NestedNameSpecifier *Prefix =
+      SS.isSet() ? SS.getScopeRep()->getPrefix() : nullptr) {
+    // This is
+    //
+    //   nested-name-specifier type-name :: ~ type-name
+    //
+    // Look for the second type-name in the nested-name-specifier.
+    CXXScopeSpec PrefixSS;
+    PrefixSS.Adopt(NestedNameSpecifierLoc(Prefix, SS.location_data()));
+    if (ParsedType T = LookupInNestedNameSpec(PrefixSS))
+      return T;
+  } else {
+    // This is one of
+    //
+    //   type-name :: ~ type-name
+    //   ~ type-name
+    //
+    // Look in the scope and (if any) the object type.
+    if (ParsedType T = LookupInScope())
+      return T;
+    if (ParsedType T = LookupInObjectType())
+      return T;
   }
 
-  if (isDependent) {
-    // We didn't find our type, but that's okay: it's dependent
-    // anyway.
+  if (Failed)
+    return nullptr;
+
+  if (IsDependent) {
+    // We didn't find our type, but that's OK: it's dependent anyway.
 
     // FIXME: What if we have no nested-name-specifier?
     QualType T = CheckTypenameType(ETK_None, SourceLocation(),
@@ -354,26 +329,98 @@ ParsedType Sema::getDestructorName(SourceLocation TildeLoc,
     return ParsedType::make(T);
   }
 
-  if (NonMatchingTypeDecl) {
-    QualType T = Context.getTypeDeclType(NonMatchingTypeDecl);
-    Diag(NameLoc, diag::err_destructor_expr_type_mismatch)
-      << T << SearchType;
-    Diag(NonMatchingTypeDecl->getLocation(), diag::note_destructor_type_here)
-      << T;
-  } else if (ObjectTypePtr)
-    Diag(NameLoc, diag::err_ident_in_dtor_not_a_type)
-      << &II;
-  else {
-    SemaDiagnosticBuilder DtorDiag = Diag(NameLoc,
-                                          diag::err_destructor_class_name);
-    if (S) {
-      const DeclContext *Ctx = S->getEntity();
-      if (const CXXRecordDecl *Class = dyn_cast_or_null<CXXRecordDecl>(Ctx))
-        DtorDiag << FixItHint::CreateReplacement(SourceRange(NameLoc),
-                                                 Class->getNameAsString());
+  // The remaining cases are all non-standard extensions imitating the behavior
+  // of various other compilers.
+  unsigned NumNonExtensionDecls = FoundDecls.size();
+
+  if (SS.isSet()) {
+    // For compatibility with older broken C++ rules and existing code,
+    //
+    //   nested-name-specifier :: ~ type-name
+    //
+    // also looks for type-name within the nested-name-specifier.
+    if (ParsedType T = LookupInNestedNameSpec(SS)) {
+      Diag(SS.getEndLoc(), diag::ext_dtor_named_in_wrong_scope)
+          << SS.getRange()
+          << FixItHint::CreateInsertion(SS.getEndLoc(),
+                                        ("::" + II.getName()).str());
+      return T;
+    }
+
+    // For compatibility with other compilers and older versions of Clang,
+    //
+    //   nested-name-specifier type-name :: ~ type-name
+    //
+    // also looks for type-name in the scope. Unfortunately, we can't
+    // reasonably apply this fallback for dependent nested-name-specifiers.
+    if (SS.getScopeRep()->getPrefix()) {
+      if (ParsedType T = LookupInScope()) {
+        Diag(SS.getEndLoc(), diag::ext_qualified_dtor_named_in_lexical_scope)
+            << FixItHint::CreateRemoval(SS.getRange());
+        Diag(FoundDecls.back()->getLocation(), diag::note_destructor_type_here)
+            << GetTypeFromParser(T);
+        return T;
+      }
     }
   }
 
+  // We didn't find anything matching; tell the user what we did find (if
+  // anything).
+
+  // Don't tell the user about declarations we shouldn't have found.
+  FoundDecls.resize(NumNonExtensionDecls);
+
+  // List types before non-types.
+  std::stable_sort(FoundDecls.begin(), FoundDecls.end(),
+                   [](NamedDecl *A, NamedDecl *B) {
+                     return isa<TypeDecl>(A->getUnderlyingDecl()) >
+                            isa<TypeDecl>(B->getUnderlyingDecl());
+                   });
+
+  // Suggest a fixit to properly name the destroyed type.
+  auto MakeFixItHint = [&]{
+    const CXXRecordDecl *Destroyed = nullptr;
+    // FIXME: If we have a scope specifier, suggest its last component?
+    if (!SearchType.isNull())
+      Destroyed = SearchType->getAsCXXRecordDecl();
+    else if (S)
+      Destroyed = dyn_cast_or_null<CXXRecordDecl>(S->getEntity());
+    if (Destroyed)
+      return FixItHint::CreateReplacement(SourceRange(NameLoc),
+                                          Destroyed->getNameAsString());
+    return FixItHint();
+  };
+
+  if (FoundDecls.empty()) {
+    // FIXME: Attempt typo-correction?
+    Diag(NameLoc, diag::err_undeclared_destructor_name)
+      << &II << MakeFixItHint();
+  } else if (!SearchType.isNull() && FoundDecls.size() == 1) {
+    if (auto *TD = dyn_cast<TypeDecl>(FoundDecls[0]->getUnderlyingDecl())) {
+      assert(!SearchType.isNull() &&
+             "should only reject a type result if we have a search type");
+      QualType T = Context.getTypeDeclType(TD);
+      Diag(NameLoc, diag::err_destructor_expr_type_mismatch)
+          << T << SearchType << MakeFixItHint();
+    } else {
+      Diag(NameLoc, diag::err_destructor_expr_nontype)
+          << &II << MakeFixItHint();
+    }
+  } else {
+    Diag(NameLoc, SearchType.isNull() ? diag::err_destructor_name_nontype
+                                      : diag::err_destructor_expr_mismatch)
+        << &II << SearchType << MakeFixItHint();
+  }
+
+  for (NamedDecl *FoundD : FoundDecls) {
+    if (auto *TD = dyn_cast<TypeDecl>(FoundD->getUnderlyingDecl()))
+      Diag(FoundD->getLocation(), diag::note_destructor_type_here)
+          << Context.getTypeDeclType(TD);
+    else
+      Diag(FoundD->getLocation(), diag::note_destructor_nontype_here)
+          << FoundD;
+  }
+
   return nullptr;
 }
 

diff  --git a/clang/test/CXX/class/class.mem/p13.cpp b/clang/test/CXX/class/class.mem/p13.cpp
index 1b1c0c7f8fc3..d947586c4194 100644
--- a/clang/test/CXX/class/class.mem/p13.cpp
+++ b/clang/test/CXX/class/class.mem/p13.cpp
@@ -110,7 +110,7 @@ template<typename B> struct Dtemplate_with_ctors : B {
 };
 
 template<typename B> struct CtorDtorName : B {
-  using B::CtorDtorName; // expected-error {{member 'CtorDtorName' has the same name as its class}}
+  using B::CtorDtorName; // expected-error {{member 'CtorDtorName' has the same name as its class}} expected-note {{non-type declaration found by destructor name lookup}}
   CtorDtorName();
-  ~CtorDtorName(); // expected-error {{expected the class name after '~' to name a destructor}}
+  ~CtorDtorName(); // expected-error {{identifier 'CtorDtorName' after '~' in destructor name does not name a type}}
 };

diff  --git a/clang/test/CXX/drs/dr2xx.cpp b/clang/test/CXX/drs/dr2xx.cpp
index 1f625efe2b55..905a2b07888d 100644
--- a/clang/test/CXX/drs/dr2xx.cpp
+++ b/clang/test/CXX/drs/dr2xx.cpp
@@ -474,45 +474,82 @@ namespace dr243 { // dr243: yes
   A a2 = b; // expected-error {{ambiguous}}
 }
 
-namespace dr244 { // dr244: partial
-  struct B {}; struct D : B {}; // expected-note {{here}}
+namespace dr244 { // dr244: 11
+  struct B {}; // expected-note {{type 'dr244::B' found by destructor name lookup}}
+  struct D : B {};
 
   D D_object;
   typedef B B_alias;
   B* B_ptr = &D_object;
 
   void f() {
-    D_object.~B(); // expected-error {{expression does not match the type}}
+    D_object.~B(); // expected-error {{does not match the type 'dr244::D' of the object being destroyed}}
     D_object.B::~B();
+    D_object.D::~B(); // FIXME: Missing diagnostic for this.
     B_ptr->~B();
     B_ptr->~B_alias();
     B_ptr->B_alias::~B();
-    // This is valid under DR244.
     B_ptr->B_alias::~B_alias();
     B_ptr->dr244::~B(); // expected-error {{refers to a member in namespace}}
     B_ptr->dr244::~B_alias(); // expected-error {{refers to a member in namespace}}
   }
 
+  template<typename T, typename U>
+  void f(T *B_ptr, U D_object) {
+    D_object.~B(); // FIXME: Missing diagnostic for this.
+    D_object.B::~B();
+    D_object.D::~B(); // FIXME: Missing diagnostic for this.
+    B_ptr->~B();
+    B_ptr->~B_alias();
+    B_ptr->B_alias::~B();
+    B_ptr->B_alias::~B_alias();
+    B_ptr->dr244::~B(); // expected-error {{does not refer to a type name}}
+    B_ptr->dr244::~B_alias(); // expected-error {{does not refer to a type name}}
+  }
+  template void f<B, D>(B*, D);
+
   namespace N {
     template<typename T> struct E {};
     typedef E<int> F;
   }
   void g(N::F f) {
-    typedef N::F G;
+    typedef N::F G; // expected-note {{found by destructor name lookup}}
     f.~G();
-    f.G::~E();
-    f.G::~F(); // expected-error {{expected the class name after '~' to name a destructor}}
+    f.G::~E(); // expected-error {{ISO C++ requires the name after '::~' to be found in the same scope as the name before '::~'}}
+    f.G::~F(); // expected-error {{undeclared identifier 'F' in destructor name}}
     f.G::~G();
     // This is technically ill-formed; E is looked up in 'N::' and names the
     // class template, not the injected-class-name of the class. But that's
     // probably a bug in the standard.
-    f.N::F::~E();
+    f.N::F::~E(); // expected-error {{ISO C++ requires the name after '::~' to be found in the same scope as the name before '::~'}}
     // This is valid; we look up the second F in the same scope in which we
     // found the first one, that is, 'N::'.
-    f.N::F::~F(); // FIXME: expected-error {{expected the class name after '~' to name a destructor}}
-    // This is technically ill-formed; G is looked up in 'N::' and is not found;
-    // as above, this is probably a bug in the standard.
-    f.N::F::~G();
+    f.N::F::~F();
+    // This is technically ill-formed; G is looked up in 'N::' and is not found.
+    // Rejecting this seems correct, but most compilers accept, so we do also.
+    f.N::F::~G(); // expected-error {{qualified destructor name only found in lexical scope; omit the qualifier to find this type name by unqualified lookup}}
+  }
+
+  // Bizarrely, compilers perform lookup in the scope for qualified destructor
+  // names, if the nested-name-specifier is non-dependent. Ensure we diagnose
+  // this.
+  namespace QualifiedLookupInScope {
+    namespace N {
+      template <typename> struct S { struct Inner {}; };
+    }
+    template <typename U> void f(typename N::S<U>::Inner *p) {
+      typedef typename N::S<U>::Inner T;
+      p->::dr244::QualifiedLookupInScope::N::S<U>::Inner::~T(); // expected-error {{no type named 'T' in}}
+    }
+    template void f<int>(N::S<int>::Inner *); // expected-note {{instantiation of}}
+
+    template <typename U> void g(U *p) {
+      typedef U T;
+      p->T::~T();
+      p->U::~T();
+      p->::dr244::QualifiedLookupInScope::N::S<int>::Inner::~T(); // expected-error {{'T' does not refer to a type name}}
+    }
+    template void g(N::S<int>::Inner *);
   }
 }
 

diff  --git a/clang/test/CXX/drs/dr3xx.cpp b/clang/test/CXX/drs/dr3xx.cpp
index d723c5b78cdf..4ce624974bbe 100644
--- a/clang/test/CXX/drs/dr3xx.cpp
+++ b/clang/test/CXX/drs/dr3xx.cpp
@@ -98,7 +98,7 @@ namespace dr305 { // dr305: no
     b->~C();
   }
   void h(B *b) {
-    struct B {}; // expected-note {{declared here}}
+    struct B {}; // expected-note {{type 'B' found by destructor name lookup}}
     b->~B(); // expected-error {{does not match}}
   }
 

diff  --git a/clang/test/FixIt/fixit.cpp b/clang/test/FixIt/fixit.cpp
index 92c561a20acc..6e3a41303af9 100644
--- a/clang/test/FixIt/fixit.cpp
+++ b/clang/test/FixIt/fixit.cpp
@@ -2,7 +2,7 @@
 // RUN: cp %s %t-98
 // RUN: not %clang_cc1 -pedantic -Wall -Wno-comment -fcxx-exceptions -fixit -x c++ -std=c++98 %t-98
 // RUN: %clang_cc1 -fsyntax-only -pedantic -Wall -Werror -Wno-comment -fcxx-exceptions -x c++ -std=c++98 %t-98
-// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -x c++ -std=c++11 %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -fsyntax-only -pedantic -fdiagnostics-parseable-fixits -x c++ -std=c++11 %s 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -pedantic -Wall -Wno-comment -verify -fcxx-exceptions -x c++ -std=c++11 %s
 // RUN: cp %s %t-11
 // RUN: not %clang_cc1 -pedantic -Wall -Wno-comment -fcxx-exceptions -fixit -x c++ -std=c++11 %t-11
@@ -318,17 +318,43 @@ class foo {
 };
 
 namespace dtor_fixit {
-  class foo {
-    ~bar() { }  // expected-error {{expected the class name after '~' to name a destructor}}
+  struct foo {
+    ~bar() { }  // expected-error {{undeclared identifier 'bar' in destructor name}}
     // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:6-[[@LINE-1]]:9}:"foo"
   };
 
-  class bar {
+  class bar { // expected-note {{found}}
     ~bar();
   };
   ~bar::bar() {} // expected-error {{'~' in destructor name should be after nested name specifier}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:4}:""
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"~"
+
+  namespace N {
+    typedef foo T;
+    template <typename T> struct X {};
+  }
+  void f(foo *p, N::X<int> *x) {
+    p->~undeclared(); // expected-error {{undeclared}}
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:19}:"foo"
+
+    p->~bar(); // expected-error {{does not match}}
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:12}:"foo"
+
+    // FIXME: This is a bad fixit; it'd be better to suggest replacing 'foo'
+    // with 'T'.
+    p->N::T::~foo(); // expected-warning {{requires the name after '::~' to be found in the same scope as the name before}}
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:12}:"::foo"
+
+    typedef foo baz; // expected-note {{found}}
+    p->dtor_fixit::foo::~baz(); // expected-warning {{only found in lexical scope}}
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:25}:""
+
+    // FIXME: This is a bad fixit; it'd be better to suggest adding the
+    // template arguments.
+    x->N::X<int>::~X(); // expected-warning {{same scope}}
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:17-[[@LINE-1]]:17}:"::X"
+  }
 }
 
 namespace PR5066 {

diff  --git a/clang/test/Parser/cxx-decl.cpp b/clang/test/Parser/cxx-decl.cpp
index 1914f347d9dd..ba1cce419a46 100644
--- a/clang/test/Parser/cxx-decl.cpp
+++ b/clang/test/Parser/cxx-decl.cpp
@@ -249,10 +249,10 @@ void foo() {
 namespace PR17567 {
   struct Foobar { // expected-note 2{{declared here}}
     FooBar(); // expected-error {{missing return type for function 'FooBar'; did you mean the constructor name 'Foobar'?}}
-    ~FooBar(); // expected-error {{expected the class name after '~' to name a destructor}}
+    ~FooBar(); // expected-error {{undeclared identifier 'FooBar' in destructor name}}
   };
   FooBar::FooBar() {} // expected-error {{undeclared}} expected-error {{missing return type}}
-  FooBar::~FooBar() {} // expected-error {{undeclared}} expected-error {{expected the class name}}
+  FooBar::~FooBar() {} // expected-error 2{{undeclared}}
 }
 
 namespace DuplicateFriend {

diff  --git a/clang/test/SemaCXX/constructor.cpp b/clang/test/SemaCXX/constructor.cpp
index 33ea49663491..d2133240cb14 100644
--- a/clang/test/SemaCXX/constructor.cpp
+++ b/clang/test/SemaCXX/constructor.cpp
@@ -94,6 +94,6 @@ namespace PR38286 {
   /*FIXME: needed to recover properly from previous error*/;
   template<typename> struct B;
   template<typename T> void B<T>::f() {} // expected-error {{out-of-line definition of 'f' from class 'B<type-parameter-0-0>'}}
-  template<typename> struct C;
-  template<typename T> C<T>::~C() {} // expected-error {{no type named 'C' in 'C<type-parameter-0-0>'}}
+  template<typename> struct C; // expected-note {{non-type declaration found}}
+  template<typename T> C<T>::~C() {} // expected-error {{identifier 'C' after '~' in destructor name does not name a type}}
 }

diff  --git a/clang/test/SemaCXX/destructor.cpp b/clang/test/SemaCXX/destructor.cpp
index 2859953a0280..92afc1256ced 100644
--- a/clang/test/SemaCXX/destructor.cpp
+++ b/clang/test/SemaCXX/destructor.cpp
@@ -75,7 +75,7 @@ struct F {
 };
 
 ~; // expected-error {{expected a class name after '~' to name a destructor}}
-~undef(); // expected-error {{expected the class name after '~' to name a destructor}}
+~undef(); // expected-error {{undeclared identifier 'undef' in destructor name}}
 ~operator+(int, int);  // expected-error {{expected a class name after '~' to name a destructor}}
 ~F(){} // expected-error {{destructor must be a non-static member function}}
 
@@ -432,7 +432,7 @@ namespace PR9238 {
 }
 
 namespace PR7900 {
-  struct A { // expected-note 2{{type 'PR7900::A' is declared here}}
+  struct A { // expected-note 2{{type 'PR7900::A' found by destructor name lookup}}
   };
   struct B : public A {
   };

diff  --git a/clang/test/SemaCXX/pseudo-destructors.cpp b/clang/test/SemaCXX/pseudo-destructors.cpp
index dfdd1174b8a4..0cd139047432 100644
--- a/clang/test/SemaCXX/pseudo-destructors.cpp
+++ b/clang/test/SemaCXX/pseudo-destructors.cpp
@@ -2,7 +2,7 @@
 struct A {};
 
 enum Foo { F };
-typedef Foo Bar; // expected-note{{type 'Bar' (aka 'Foo') is declared here}}
+typedef Foo Bar; // expected-note{{type 'Bar' (aka 'Foo') found by destructor name lookup}}
 
 typedef int Integer;
 typedef double Double;
@@ -23,7 +23,7 @@ void f(A* a, Foo *f, int *i, double *d, int ii) {
   a->~A();
   a->A::~A();
   
-  a->~foo(); // expected-error{{identifier 'foo' in object destruction expression does not name a type}}
+  a->~foo(); // expected-error{{undeclared identifier 'foo' in destructor name}}
   
   a->~Bar(); // expected-error{{destructor type 'Bar' (aka 'Foo') in object destruction expression does not match the type 'A' of the object being destroyed}}
   
@@ -83,7 +83,7 @@ namespace PR11339 {
   template<class T>
   void destroy(T* p) {
     p->~T(); // ok
-    p->~oops(); // expected-error{{identifier 'oops' in object destruction expression does not name a type}}
+    p->~oops(); // expected-error{{undeclared identifier 'oops' in destructor name}}
   }
 
   template void destroy(int*); // expected-note{{in instantiation of function template specialization}}


        


More information about the cfe-commits mailing list