[clang] [Clang][Sema] Fix exception specification comparison for functions with different template depths (PR #111561)
Krystian Stasiowski via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 22 09:22:37 PDT 2024
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/111561
>From d41114109ebfb98570152fe8fdc2854863f23397 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Tue, 8 Oct 2024 12:54:26 -0400
Subject: [PATCH 1/4] [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 bc9c422ed4c477..b295db62bf016d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5026,6 +5026,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 7446198d879c4aabec3ae4b2d23ab3a50560c7cc Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Tue, 8 Oct 2024 14:13:59 -0400
Subject: [PATCH 2/4] [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 1be01ebe5051b1e34a5a15936ce7e7e39a3b2761 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 18 Oct 2024 11:36:00 -0400
Subject: [PATCH 3/4] [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);
>From 7df326227711c3247ca2b52c1d86ceb33eac8629 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 18 Oct 2024 13:18:49 -0400
Subject: [PATCH 4/4] [FOLD] fix typo
---
clang/lib/Sema/SemaExceptionSpec.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index 0f000b4da5f660..a532c0a2c84e0c 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -320,7 +320,7 @@ bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) {
New->getExceptionSpecType() == EST_DependentNoexcept) {
const auto *OldType = Old->getType()->getAs<FunctionProtoType>();
const auto *NewType = New->getType()->getAs<FunctionProtoType>();
- OldType = ResolveExceptionSpec(New->getLocation(), OldType);
+ OldType = ResolveExceptionSpec(Old->getLocation(), OldType);
if (!OldType)
return false;
NewType = ResolveExceptionSpec(New->getLocation(), NewType);
More information about the cfe-commits
mailing list