On Sun, 9 Feb 2020 at 11:33, Richard Smith <rich...@metafoo.co.uk> wrote:
> On Sun, 9 Feb 2020, 01:09 Nico Weber via cfe-commits, < > cfe-commits@lists.llvm.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 you provide a sketch of what you were doing? There are certainly cases > where I'd expect that now -- where you find a typedef through in "bad" > (extension) place and find a non-typedef elsewhere. > I found that we were rejecting potentially-valid code in some cases: struct X { ~X(); }; using X = X; ::X::~X() {} // rejected because we find the typedef-name not the class-name Fixed in llvmorg-11-init-2613-g76f888d0a53. > Can this be a default-error-mapped warning so that projects have some >> incremental transition path for this? >> > > That seems reasonable, yes. > Now accepted as an error-by-default extension (-Wno-dtor-typedef to disable the error), in the same commit. (This doesn't have a fixit hint yet, though.) > 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 >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits