On Fri, 6 Nov 2020 at 13:30, Alex L <arpha...@gmail.com> wrote: > Hi Rchard, > > Some of our code started triggering the warning you added in this code, > but I'm not sure if the warning is actually valid or not in this case: > > https://godbolt.org/z/9fxs3K > > namespace geo { > template<typename T> > class optional { > optional() { } > ~optional(); > }; > } > template<typename T> > geo::optional<T>::~optional() // Triggers ISO C++ requires the name after > '::~' to be found in the same scope as the name before '::~' > { > } > > Could you confirm whether or not this warning is valid for our code? >
Technically, yes, the warning is correct; the C++ rules require that to be written as geo::optional<T>::~optional<T>() ... but of course no-one does that, and it doesn't seem to be an intended restriction. I've been working with the C++ committee to get the language rule fixed, and the fix will be in P1787R6, which is due to be voted into C++23 (with at least this part as a DR against prior standards) next week. It's probably time to implement the new rule :) In the meantime, using -Wno-dtor-name would be reasonable. > Thanks, > Alex > > > On Thu, 13 Feb 2020 at 00:52, Richard Smith via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Thanks, yes, fixed in llvmorg-11-init-3043-gc1394afb8df. >> >> On Thu, 13 Feb 2020 at 02:58, Kostya Serebryany via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> Could this have caused a new ubsan failure? >>> >>> clang/lib/AST/NestedNameSpecifier.cpp:485:23: runtime error: null pointer >>> passed as argument 2, which is declared to never be null >>> >>> >>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/38698/steps/check-clang%20ubsan/logs/stdio >>> >>> On Fri, Feb 7, 2020 at 6: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 >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
