From 7a80dab9bb96cff16eba62318cca244ac7029374 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Tue, 18 Mar 2025 17:02:45 -0300 Subject: [PATCH] [clang] ASTContext: flesh out implementation of getCommonNNS This properly implements getCommonNNS, for getting the common NestedNameSpecifier, for which the previous implementation was a bare minimum placeholder. --- clang/docs/ReleaseNotes.rst | 2 + clang/lib/AST/ASTContext.cpp | 167 ++++++++++++++++++++-- clang/test/SemaCXX/sugar-common-types.cpp | 4 +- 3 files changed, 160 insertions(+), 13 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 9b0680c68b83a..4258d0d72c950 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -271,6 +271,8 @@ Improvements to Clang's diagnostics as function arguments or return value respectively. Note that :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The feature will be default-enabled with ``-Wthread-safety`` in a future release. +- Clang will now do a better job producing common nested names, when producing + common types for ternary operator, template argument deduction and multiple return auto deduction. - The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) and minus(-) as signed integers except for the case where the operand is an unsigned integer and throws warning if they are compared with unsigned integers (##18878). diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 7ed5b033d9bd8..68a02f3bbe1ec 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -13462,13 +13462,155 @@ static ElaboratedTypeKeyword getCommonTypeKeyword(const T *X, const T *Y) { : ElaboratedTypeKeyword::None; } +/// Returns a NestedNameSpecifier which has only the common sugar +/// present in both NNS1 and NNS2. +static NestedNameSpecifier *getCommonNNS(ASTContext &Ctx, + NestedNameSpecifier *NNS1, + NestedNameSpecifier *NNS2, + bool IsSame) { + // If they are identical, all sugar is common. + if (NNS1 == NNS2) + return NNS1; + + // IsSame implies both NNSes are equivalent. + NestedNameSpecifier *Canon = Ctx.getCanonicalNestedNameSpecifier(NNS1); + if (Canon != Ctx.getCanonicalNestedNameSpecifier(NNS2)) { + assert(!IsSame && "Should be the same NestedNameSpecifier"); + // If they are not the same, there is nothing to unify. + // FIXME: It would be useful here if we could represent a canonically + // empty NNS, which is not identical to an empty-as-written NNS. + return nullptr; + } + + NestedNameSpecifier *R = nullptr; + NestedNameSpecifier::SpecifierKind K1 = NNS1->getKind(), K2 = NNS2->getKind(); + switch (K1) { + case NestedNameSpecifier::SpecifierKind::Identifier: { + assert(K2 == NestedNameSpecifier::SpecifierKind::Identifier); + IdentifierInfo *II = NNS1->getAsIdentifier(); + assert(II == NNS2->getAsIdentifier()); + // For an identifier, the prefixes are significant, so they must be the + // same. + NestedNameSpecifier *P = ::getCommonNNS(Ctx, NNS1->getPrefix(), + NNS2->getPrefix(), /*IsSame=*/true); + R = NestedNameSpecifier::Create(Ctx, P, II); + break; + } + case NestedNameSpecifier::SpecifierKind::Namespace: + case NestedNameSpecifier::SpecifierKind::NamespaceAlias: { + assert(K2 == NestedNameSpecifier::SpecifierKind::Namespace || + K2 == NestedNameSpecifier::SpecifierKind::NamespaceAlias); + // The prefixes for namespaces are not significant, its declaration + // identifies it uniquely. + NestedNameSpecifier *P = + ::getCommonNNS(Ctx, NNS1->getPrefix(), NNS2->getPrefix(), + /*IsSame=*/false); + NamespaceAliasDecl *A1 = NNS1->getAsNamespaceAlias(), + *A2 = NNS2->getAsNamespaceAlias(); + // Are they the same namespace alias? + if (declaresSameEntity(A1, A2)) { + R = NestedNameSpecifier::Create(Ctx, P, ::getCommonDeclChecked(A1, A2)); + break; + } + // Otherwise, look at the namespaces only. + NamespaceDecl *N1 = A1 ? A1->getNamespace() : NNS1->getAsNamespace(), + *N2 = A2 ? A2->getNamespace() : NNS2->getAsNamespace(); + R = NestedNameSpecifier::Create(Ctx, P, ::getCommonDeclChecked(N1, N2)); + break; + } + case NestedNameSpecifier::SpecifierKind::TypeSpec: + case NestedNameSpecifier::SpecifierKind::TypeSpecWithTemplate: { + // FIXME: See comment below, on Super case. + if (K2 == NestedNameSpecifier::SpecifierKind::Super) + return Ctx.getCanonicalNestedNameSpecifier(NNS1); + + assert(K2 == NestedNameSpecifier::SpecifierKind::TypeSpec || + K2 == NestedNameSpecifier::SpecifierKind::TypeSpecWithTemplate); + + // Only keep the template keyword if both sides have it. + bool Template = + K1 == NestedNameSpecifier::SpecifierKind::TypeSpecWithTemplate && + K2 == NestedNameSpecifier::SpecifierKind::TypeSpecWithTemplate; + + const Type *T1 = NNS1->getAsType(), *T2 = NNS2->getAsType(); + if (T1 == T2) { + // If the types are indentical, then only the prefixes differ. + // A well-formed NNS never has these types, as they have + // special normalized forms. + assert((!isa(T1))); + // Only for a DependentTemplateSpecializationType the prefix + // is actually significant. A DependentName, which would be another + // plausible case, cannot occur here, as explained above. + bool IsSame = isa(T1); + NestedNameSpecifier *P = + ::getCommonNNS(Ctx, NNS1->getPrefix(), NNS2->getPrefix(), IsSame); + R = NestedNameSpecifier::Create(Ctx, P, Template, T1); + break; + } + // TODO: Try to salvage the original prefix. + // If getCommonSugaredType removed any top level sugar, the original prefix + // is not applicable anymore. + NestedNameSpecifier *P = nullptr; + const Type *T = Ctx.getCommonSugaredType(QualType(T1, 0), QualType(T2, 0), + /*Unqualified=*/true) + .getTypePtr(); + + // A NestedNameSpecifier has special normalization rules for certain types. + switch (T->getTypeClass()) { + case Type::Elaborated: { + // An ElaboratedType is stripped off, it's Qualifier becomes the prefix. + auto *ET = cast(T); + R = NestedNameSpecifier::Create(Ctx, ET->getQualifier(), Template, + ET->getNamedType().getTypePtr()); + break; + } + case Type::DependentName: { + // A DependentName is turned into an Identifier NNS. + auto *DN = cast(T); + R = NestedNameSpecifier::Create(Ctx, DN->getQualifier(), + DN->getIdentifier()); + break; + } + case Type::DependentTemplateSpecialization: { + // A DependentTemplateSpecializationType loses it's Qualifier, which + // is turned into the prefix. + auto *DTST = cast(T); + T = Ctx.getDependentTemplateSpecializationType( + DTST->getKeyword(), /*NNS=*/nullptr, DTST->getIdentifier(), + DTST->template_arguments()) + .getTypePtr(); + P = DTST->getQualifier(); + R = NestedNameSpecifier::Create(Ctx, DTST->getQualifier(), Template, T); + break; + } + default: + R = NestedNameSpecifier::Create(Ctx, P, Template, T); + break; + } + break; + } + case NestedNameSpecifier::SpecifierKind::Super: + // FIXME: Can __super even be used with data members? + // If it's only usable in functions, we will never see it here, + // unless we save the qualifiers used in function types. + // In that case, it might be possible NNS2 is a type, + // in which case we should degrade the result to + // a CXXRecordType. + return Ctx.getCanonicalNestedNameSpecifier(NNS1); + case NestedNameSpecifier::SpecifierKind::Global: + // The global NNS is a singleton. + assert(K2 == NestedNameSpecifier::SpecifierKind::Global && + "Global NNS cannot be equivalent to any other kind"); + llvm_unreachable("Global NestedNameSpecifiers did not compare equal"); + } + assert(Ctx.getCanonicalNestedNameSpecifier(R) == Canon); + return R; +} + template -static NestedNameSpecifier *getCommonNNS(ASTContext &Ctx, const T *X, - const T *Y) { - // FIXME: Try to keep the common NNS sugar. - return X->getQualifier() == Y->getQualifier() - ? X->getQualifier() - : Ctx.getCanonicalNestedNameSpecifier(X->getQualifier()); +static NestedNameSpecifier *getCommonQualifier(ASTContext &Ctx, const T *X, + const T *Y, bool IsSame) { + return ::getCommonNNS(Ctx, X->getQualifier(), Y->getQualifier(), IsSame); } template @@ -13879,8 +14021,9 @@ static QualType getCommonNonSugarTypeNode(ASTContext &Ctx, const Type *X, *NY = cast(Y); assert(NX->getIdentifier() == NY->getIdentifier()); return Ctx.getDependentNameType( - getCommonTypeKeyword(NX, NY), getCommonNNS(Ctx, NX, NY), - NX->getIdentifier(), NX->getCanonicalTypeInternal()); + getCommonTypeKeyword(NX, NY), + getCommonQualifier(Ctx, NX, NY, /*IsSame=*/true), NX->getIdentifier(), + NX->getCanonicalTypeInternal()); } case Type::DependentTemplateSpecialization: { const auto *TX = cast(X), @@ -13889,8 +14032,9 @@ static QualType getCommonNonSugarTypeNode(ASTContext &Ctx, const Type *X, auto As = getCommonTemplateArguments(Ctx, TX->template_arguments(), TY->template_arguments()); return Ctx.getDependentTemplateSpecializationType( - getCommonTypeKeyword(TX, TY), getCommonNNS(Ctx, TX, TY), - TX->getIdentifier(), As); + getCommonTypeKeyword(TX, TY), + getCommonQualifier(Ctx, TX, TY, /*IsSame=*/true), TX->getIdentifier(), + As); } case Type::UnaryTransform: { const auto *TX = cast(X), @@ -14047,7 +14191,8 @@ static QualType getCommonSugarTypeNode(ASTContext &Ctx, const Type *X, case Type::Elaborated: { const auto *EX = cast(X), *EY = cast(Y); return Ctx.getElaboratedType( - ::getCommonTypeKeyword(EX, EY), ::getCommonNNS(Ctx, EX, EY), + ::getCommonTypeKeyword(EX, EY), + ::getCommonQualifier(Ctx, EX, EY, /*IsSame=*/false), Ctx.getQualifiedType(Underlying), ::getCommonDecl(EX->getOwnedTagDecl(), EY->getOwnedTagDecl())); } diff --git a/clang/test/SemaCXX/sugar-common-types.cpp b/clang/test/SemaCXX/sugar-common-types.cpp index 39a762127811f..3c4de0e2abd5e 100644 --- a/clang/test/SemaCXX/sugar-common-types.cpp +++ b/clang/test/SemaCXX/sugar-common-types.cpp @@ -44,7 +44,7 @@ template struct S1 { }; N t10 = 0 ? S1() : S1(); // expected-error {{from 'S1' (aka 'S1')}} -N t11 = 0 ? S1::S2() : S1::S2(); // expected-error {{from 'S1::S2' (aka 'S2')}} +N t11 = 0 ? S1::S2() : S1::S2(); // expected-error {{from 'S1::S2' (aka 'S2')}} template using Al = S1; @@ -88,7 +88,7 @@ N t19 = 0 ? (__underlying_type(EnumsX::X)){} : (__underlying_type(EnumsY::Y)){}; // expected-error@-1 {{rvalue of type 'B1' (aka 'int')}} N t20 = 0 ? (__underlying_type(EnumsX::X)){} : (__underlying_type(EnumsY::X)){}; -// expected-error@-1 {{rvalue of type '__underlying_type(Enums::X)' (aka 'int')}} +// expected-error@-1 {{rvalue of type '__underlying_type(EnumsB::X)' (aka 'int')}} using QX = const SB1 *; using QY = const ::SB1 *;