(Also, it'd be nice if the "destructor cannot be declared using a type alias" diag had a fixit attached to it :) )
On Sat, Feb 8, 2020 at 7:09 PM Nico Weber <tha...@chromium.org> wrote: > Our code fails to build with "destructor cannot be declared using a type > alias" after this, without us changing language mode or anything. > > Is that intended? Can this be a default-error-mapped warning so that > projects have some incremental transition path for this? > > On Fri, Feb 7, 2020 at 9:41 PM Richard Smith via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> >> 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}} >> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
