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

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 13 00:52:07 PST 2020


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200213/d8c70d95/attachment-0001.html>


More information about the cfe-commits mailing list