https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/111561
>From dd1095566251508aae4009a626774413eb8dc7a1 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski <sdkryst...@gmail.com> Date: Tue, 8 Oct 2024 12:54:26 -0400 Subject: [PATCH 1/3] [Clang][Sema] Fix exception specification comparison for functions with different template depths --- clang/include/clang/Sema/Sema.h | 5 ++ clang/lib/Sema/SemaExceptionSpec.cpp | 105 +++++++++++++++++++++++- clang/test/CXX/basic/basic.link/p11.cpp | 37 +++++++++ 3 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 clang/test/CXX/basic/basic.link/p11.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 0faa5aed4eec3b..9ae882f651e2b0 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5028,6 +5028,11 @@ class Sema final : public SemaBase { /// special member function. void EvaluateImplicitExceptionSpec(SourceLocation Loc, FunctionDecl *FD); + bool AreExceptionSpecsEqual(const NamedDecl *Old, + const Expr *OldExceptionSpec, + const NamedDecl *New, + const Expr *NewExceptionSpec); + /// Check the given exception-specification and update the /// exception specification information with the results. void checkExceptionSpecification(bool IsTopLevel, diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp index dbddd6c370aa07..c74686073df228 100644 --- a/clang/lib/Sema/SemaExceptionSpec.cpp +++ b/clang/lib/Sema/SemaExceptionSpec.cpp @@ -10,7 +10,6 @@ // //===----------------------------------------------------------------------===// -#include "clang/Sema/SemaInternal.h" #include "clang/AST/ASTMutationListener.h" #include "clang/AST/CXXInheritance.h" #include "clang/AST/Expr.h" @@ -19,6 +18,9 @@ #include "clang/AST/TypeLoc.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/SourceManager.h" +#include "clang/Sema/EnterExpressionEvaluationContext.h" +#include "clang/Sema/SemaInternal.h" +#include "clang/Sema/Template.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallString.h" #include <optional> @@ -314,6 +316,22 @@ bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) { return false; } + if (Old->getExceptionSpecType() == EST_DependentNoexcept && + New->getExceptionSpecType() == EST_DependentNoexcept) { + const auto *OldType = Old->getType()->getAs<FunctionProtoType>(); + const auto *NewType = New->getType()->getAs<FunctionProtoType>(); + OldType = ResolveExceptionSpec(New->getLocation(), OldType); + if (!OldType) + return false; + NewType = ResolveExceptionSpec(New->getLocation(), NewType); + if (!NewType) + return false; + + if (AreExceptionSpecsEqual(Old, OldType->getNoexceptExpr(), New, + NewType->getNoexceptExpr())) + return false; + } + // Check the types as written: they must match before any exception // specification adjustment is applied. if (!CheckEquivalentExceptionSpecImpl( @@ -501,6 +519,89 @@ bool Sema::CheckEquivalentExceptionSpec( return Result; } +static const Expr *SubstituteExceptionSpecWithoutEvaluation( + Sema &S, const Sema::TemplateCompareNewDeclInfo &DeclInfo, + const Expr *ExceptionSpec) { + MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs( + DeclInfo.getDecl(), DeclInfo.getLexicalDeclContext(), + /*Final=*/false, /*Innermost=*/std::nullopt, + /*RelativeToPrimary=*/true, /*ForConstraintInstantiation=*/true); + + if (!MLTAL.getNumSubstitutedLevels()) + return ExceptionSpec; + + Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false); + + Sema::InstantiatingTemplate Inst( + S, DeclInfo.getLocation(), + const_cast<FunctionDecl *>(DeclInfo.getDecl()->getAsFunction()), + Sema::InstantiatingTemplate::ExceptionSpecification()); + if (Inst.isInvalid()) + return nullptr; + + // Set up a dummy 'instantiation' scope in the case of reference to function + // parameters that the surrounding function hasn't been instantiated yet. Note + // this may happen while we're comparing two templates' constraint + // equivalence. + LocalInstantiationScope ScopeForParameters(S); + if (auto *FD = DeclInfo.getDecl()->getAsFunction()) + for (auto *PVD : FD->parameters()) + ScopeForParameters.InstantiatedLocal(PVD, PVD); + + std::optional<Sema::CXXThisScopeRAII> ThisScope; + + // See TreeTransform::RebuildTemplateSpecializationType. A context scope is + // essential for having an injected class as the canonical type for a template + // specialization type at the rebuilding stage. This guarantees that, for + // out-of-line definitions, injected class name types and their equivalent + // template specializations can be profiled to the same value, which makes it + // possible that e.g. constraints involving C<Class<T>> and C<Class> are + // perceived identical. + std::optional<Sema::ContextRAII> ContextScope; + if (auto *RD = dyn_cast<CXXRecordDecl>(DeclInfo.getDeclContext())) { + ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers()); + ContextScope.emplace(S, const_cast<DeclContext *>(cast<DeclContext>(RD)), + /*NewThisContext=*/false); + } + + EnterExpressionEvaluationContext ConstantEvaluated( + S, Sema::ExpressionEvaluationContext::ConstantEvaluated); + + ExprResult SubstExceptionSpec = + S.SubstExpr(const_cast<clang::Expr *>(ExceptionSpec), MLTAL); + if (SFINAE.hasErrorOccurred() || !SubstExceptionSpec.isUsable()) + return nullptr; + return SubstExceptionSpec.get(); +} + +bool Sema::AreExceptionSpecsEqual(const NamedDecl *Old, + const Expr *OldExceptionSpec, + const NamedDecl *New, + const Expr *NewExceptionSpec) { + if (OldExceptionSpec == NewExceptionSpec) + return true; + if (Old && New && + Old->getLexicalDeclContext() != New->getLexicalDeclContext()) { + if (const Expr *SubstExceptionSpec = + SubstituteExceptionSpecWithoutEvaluation(*this, Old, + OldExceptionSpec)) + OldExceptionSpec = SubstExceptionSpec; + else + return false; + if (const Expr *SubstExceptionSpec = + SubstituteExceptionSpecWithoutEvaluation(*this, New, + NewExceptionSpec)) + NewExceptionSpec = SubstExceptionSpec; + else + return false; + } + + llvm::FoldingSetNodeID ID1, ID2; + OldExceptionSpec->Profile(ID1, Context, /*Canonical=*/true); + NewExceptionSpec->Profile(ID2, Context, /*Canonical=*/true); + return ID1 == ID2; +} + /// CheckEquivalentExceptionSpec - Check if the two types have compatible /// exception specifications. See C++ [except.spec]p3. /// @@ -574,6 +675,7 @@ static bool CheckEquivalentExceptionSpecImpl( } } +#if 0 // C++14 [except.spec]p3: // Two exception-specifications are compatible if [...] both have the form // noexcept(constant-expression) and the constant-expressions are equivalent @@ -584,6 +686,7 @@ static bool CheckEquivalentExceptionSpecImpl( if (OldFSN == NewFSN) return false; } +#endif // Dynamic exception specifications with the same set of adjusted types // are compatible. diff --git a/clang/test/CXX/basic/basic.link/p11.cpp b/clang/test/CXX/basic/basic.link/p11.cpp new file mode 100644 index 00000000000000..e244336417fd67 --- /dev/null +++ b/clang/test/CXX/basic/basic.link/p11.cpp @@ -0,0 +1,37 @@ +// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s + +namespace MemberSpecialization { + template<typename T> + struct A { + template<bool B> + void f() noexcept(B); + + template<bool B> + void g() noexcept(B); // expected-note {{previous declaration is here}} + }; + + template<> + template<bool B> + void A<int>::f() noexcept(B); + + template<> + template<bool B> + void A<int>::g() noexcept(!B); // expected-error {{exception specification in declaration does not match previous declaration}} +} + +namespace Friend { + template<bool B> + void f() noexcept(B); + + template<bool B> + void g() noexcept(B); // expected-note {{previous declaration is here}} + + template<typename T> + struct A { + template<bool B> + friend void f() noexcept(B); + + template<bool B> + friend void g() noexcept(!B); // expected-error {{exception specification in declaration does not match previous declaration}} + }; +} >From cac5401cd34e35dc4fefe82233194adbb2b7eb6b Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski <sdkryst...@gmail.com> Date: Tue, 8 Oct 2024 14:13:59 -0400 Subject: [PATCH 2/3] [FOLD] remove dead code --- clang/lib/Sema/SemaExceptionSpec.cpp | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp index c74686073df228..9ef096ca13e6b4 100644 --- a/clang/lib/Sema/SemaExceptionSpec.cpp +++ b/clang/lib/Sema/SemaExceptionSpec.cpp @@ -675,19 +675,6 @@ static bool CheckEquivalentExceptionSpecImpl( } } -#if 0 - // C++14 [except.spec]p3: - // Two exception-specifications are compatible if [...] both have the form - // noexcept(constant-expression) and the constant-expressions are equivalent - if (OldEST == EST_DependentNoexcept && NewEST == EST_DependentNoexcept) { - llvm::FoldingSetNodeID OldFSN, NewFSN; - Old->getNoexceptExpr()->Profile(OldFSN, S.Context, true); - New->getNoexceptExpr()->Profile(NewFSN, S.Context, true); - if (OldFSN == NewFSN) - return false; - } -#endif - // Dynamic exception specifications with the same set of adjusted types // are compatible. if (OldEST == EST_Dynamic && NewEST == EST_Dynamic) { >From 45a2d494bf60f8a4ce6153941b2439c8f4d117a3 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski <sdkryst...@gmail.com> Date: Fri, 18 Oct 2024 11:36:00 -0400 Subject: [PATCH 3/3] [FOLD] simplify SubstituteExceptionSpecWithoutEvaluation --- clang/lib/Sema/SemaExceptionSpec.cpp | 53 +++++++++++++++++++--------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp index 9ef096ca13e6b4..0f000b4da5f660 100644 --- a/clang/lib/Sema/SemaExceptionSpec.cpp +++ b/clang/lib/Sema/SemaExceptionSpec.cpp @@ -523,18 +523,19 @@ static const Expr *SubstituteExceptionSpecWithoutEvaluation( Sema &S, const Sema::TemplateCompareNewDeclInfo &DeclInfo, const Expr *ExceptionSpec) { MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs( - DeclInfo.getDecl(), DeclInfo.getLexicalDeclContext(), - /*Final=*/false, /*Innermost=*/std::nullopt, - /*RelativeToPrimary=*/true, /*ForConstraintInstantiation=*/true); + DeclInfo.getDecl(), DeclInfo.getLexicalDeclContext(), /*Final=*/false, + /*Innermost=*/std::nullopt, + /*RelativeToPrimary=*/true, + /*ForConstraintInstantiation=*/true); - if (!MLTAL.getNumSubstitutedLevels()) + if (MLTAL.getNumSubstitutedLevels() == 0) return ExceptionSpec; Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false); + auto *FD = const_cast<FunctionDecl *>(DeclInfo.getDecl()->getAsFunction()); Sema::InstantiatingTemplate Inst( - S, DeclInfo.getLocation(), - const_cast<FunctionDecl *>(DeclInfo.getDecl()->getAsFunction()), + S, DeclInfo.getLocation(), FD, Sema::InstantiatingTemplate::ExceptionSpecification()); if (Inst.isInvalid()) return nullptr; @@ -544,11 +545,31 @@ static const Expr *SubstituteExceptionSpecWithoutEvaluation( // this may happen while we're comparing two templates' constraint // equivalence. LocalInstantiationScope ScopeForParameters(S); - if (auto *FD = DeclInfo.getDecl()->getAsFunction()) - for (auto *PVD : FD->parameters()) - ScopeForParameters.InstantiatedLocal(PVD, PVD); - std::optional<Sema::CXXThisScopeRAII> ThisScope; + for (auto *PVD : FD->parameters()) { + if (!PVD->isParameterPack()) { + ScopeForParameters.InstantiatedLocal(PVD, PVD); + continue; + } + // This is hacky: we're mapping the parameter pack to a size-of-1 argument + // to avoid building SubstTemplateTypeParmPackTypes for + // PackExpansionTypes. The SubstTemplateTypeParmPackType node would + // otherwise reference the AssociatedDecl of the template arguments, which + // is, in this case, the template declaration. + // + // However, as we are in the process of comparing potential + // re-declarations, the canonical declaration is the declaration itself at + // this point. So if we didn't expand these packs, we would end up with an + // incorrect profile difference because we will be profiling the + // canonical types! + // + // FIXME: Improve the "no-transform" machinery in FindInstantiatedDecl so + // that we can eliminate the Scope in the cases where the declarations are + // not necessarily instantiated. It would also benefit the noexcept + // specifier comparison. + ScopeForParameters.MakeInstantiatedLocalArgPack(PVD); + ScopeForParameters.InstantiatedLocalPackArg(PVD, PVD); + } // See TreeTransform::RebuildTemplateSpecializationType. A context scope is // essential for having an injected class as the canonical type for a template @@ -557,12 +578,12 @@ static const Expr *SubstituteExceptionSpecWithoutEvaluation( // template specializations can be profiled to the same value, which makes it // possible that e.g. constraints involving C<Class<T>> and C<Class> are // perceived identical. - std::optional<Sema::ContextRAII> ContextScope; - if (auto *RD = dyn_cast<CXXRecordDecl>(DeclInfo.getDeclContext())) { - ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers()); - ContextScope.emplace(S, const_cast<DeclContext *>(cast<DeclContext>(RD)), - /*NewThisContext=*/false); - } + Sema::ContextRAII ContextScope(S, FD); + + auto *MD = dyn_cast<CXXMethodDecl>(FD); + Sema::CXXThisScopeRAII ThisScope( + S, MD ? MD->getParent() : nullptr, + MD ? MD->getMethodQualifiers() : Qualifiers{}, MD != nullptr); EnterExpressionEvaluationContext ConstantEvaluated( S, Sema::ExpressionEvaluationContext::ConstantEvaluated); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits