[clang] [Clang][Sema] Fix exception specification comparison for functions with different template depths (PR #111561)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 8 10:11:14 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Krystian Stasiowski (sdkrystian)
<details>
<summary>Changes</summary>
Currently, we do not account for differences in template depth when comparing exception specifications for equivalence. This results in explicit specializations of member function templates specialized for an implicitly instantiated specialization of their enclosing class template & friend declarations being rejected when their exception specifications involve a template parameter, e.g.
```cpp
template<typename T>
struct A {
template<bool B>
void f() noexcept(B);
};
template<>
template<bool B>
void A<int>::f() noexcept(B); // error: exception specification in declaration does not match previous declaration
```
In order to compare the _noexcept-specifiers_ correctly, we need to normalize both expressions prior to comparing them (just like we do when comparing _constraint-expressions_). This patch implements this normalization.
Note: In its current state, this patch just copies the code from `AreConstraintExpressionsEqual` to implement expression normalization for _noexcept-specifiers_. I plan to factor out the common code from `AreConstraintExpressionsEqual` and `AreExceptionSpecsEqual` prior to merging this patch.
Fixes #<!-- -->101330, closes #<!-- -->102267
---
Full diff: https://github.com/llvm/llvm-project/pull/111561.diff
3 Files Affected:
- (modified) clang/include/clang/Sema/Sema.h (+5)
- (modified) clang/lib/Sema/SemaExceptionSpec.cpp (+104-1)
- (added) clang/test/CXX/basic/basic.link/p11.cpp (+37)
``````````diff
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 7ff9c2754a6fe0..eea950582929d9 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}}
+ };
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/111561
More information about the cfe-commits
mailing list