[clang] 0e3a487 - PR12350: Handle remaining cases permitted by CWG DR 244.
Alex L via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 6 13:30:11 PST 2020
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?
Thanks,
Alex
On Thu, 13 Feb 2020 at 00:52, Richard Smith via cfe-commits <
cfe-commits at 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 at 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 at 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 at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201106/62ef4126/attachment-0001.html>
More information about the cfe-commits
mailing list