[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 8 10:10:38 PDT 2024


https://github.com/sdkrystian created https://github.com/llvm/llvm-project/pull/111561

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

>From da01bdac8a99a127a74073df4a505bdc936291e2 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] [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 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}}
+  };
+}



More information about the cfe-commits mailing list