[Clang] Don't retain template FoundDecl for conversion function calls

[zyn0217]

In consteval calls rewriting, we expect an instantiated AST to remain untouched. However, this was not true when rebuilding a MemberExpr in TreeTransform, as it relies on FoundDecl to build a resolved member call expression. This is due to the fact that the instantiated CXXMemberCallExpr still holds a template declaration as FoundDecl, leading to a non-dependent-to-dependent transform.

I don't think it makes much sense to preserve a template in FoundDecl. FoundDecl should primarily distinguish declarations whose getUnderlyingDecl() points elsewhere, like UsingShadowDecl.

Fixes #137885

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

In consteval calls rewriting, we expect an instantiated AST to remain untouched. However, this was not true when rebuilding a MemberExpr in TreeTransform, as it relies on FoundDecl to build a resolved member call expression. This is due to the fact that the instantiated CXXMemberCallExpr still holds a template declaration as FoundDecl, leading to a non-dependent-to-dependent transform.

I don't think it makes much sense to preserve a template in FoundDecl. FoundDecl should primarily distinguish declarations whose getUnderlyingDecl() points elsewhere, like UsingShadowDecl.

Fixes #137885

---

Full diff: https://github.com/llvm/llvm-project/pull/138377.diff

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/Sema/SemaOverload.cpp (+7-1)
• (modified) clang/test/SemaCXX/cxx2a-consteval.cpp (+28)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5ccd346a93b4f..5e26a3052452b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -545,6 +545,7 @@ Bug Fixes to C++ Support
 - Clang no longer crashes when establishing subsumption between some constraint expressions. (#GH122581)
 - Clang now issues an error when placement new is used to modify a const-qualified variable
   in a ``constexpr`` function. (#GH131432)
+- Fixed an incorrect TreeTransform for calls to ``consteval`` functions if a conversion template is present. (#GH137885)
 - Clang now emits a warning when class template argument deduction for alias templates is used in C++17. (#GH133806)
 
 Bug Fixes to AST Handling
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 5b224b6c08fef..05707b964bc27 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -3977,7 +3977,7 @@ IsUserDefinedConversion(Sema &S, Expr *From, QualType ToType,
           //   phase of [over.match.list] when the initializer list has exactly
           //   one element that is itself an initializer list, [...] and the
           //   conversion is to X or reference to cv X, user-defined conversion
-          //   sequences are not cnosidered.
+          //   sequences are not considered.
           if (SuppressUserConversions && ListInitializing) {
             SuppressUserConversions =
                 NumArgs == 1 && isa<InitListExpr>(Args[0]) &&
@@ -14750,6 +14750,12 @@ ExprResult Sema::CreateUnresolvedLookupExpr(CXXRecordDecl *NamingClass,
 ExprResult Sema::BuildCXXMemberCallExpr(Expr *E, NamedDecl *FoundDecl,
                                         CXXConversionDecl *Method,
                                         bool HadMultipleCandidates) {
+  // FoundDecl can be the TemplateDecl of Method. Don't retain a template in
+  // the FoundDecl as it impedes TransformMemberExpr.
+  // We go a bit further here: if there's no difference in UnderlyingDecl,
+  // then using FoundDecl vs Method shouldn't make a difference either.
+  if (FoundDecl->getUnderlyingDecl() == FoundDecl)
+    FoundDecl = Method;
   // Convert the expression to match the conversion function's implicit object
   // parameter.
   ExprResult Exp;
diff --git a/clang/test/SemaCXX/cxx2a-consteval.cpp b/clang/test/SemaCXX/cxx2a-consteval.cpp
index fef4674d17841..d9d144cafdbcc 100644
--- a/clang/test/SemaCXX/cxx2a-consteval.cpp
+++ b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -1272,3 +1272,31 @@ int main() {
 }
 
 } // namespace GH107175
+
+namespace GH137885 {
+
+template <typename... P> struct A {};
+
+template <int N>
+struct B {
+  consteval B() {}
+
+  template <typename... P> consteval operator A<P...>() const {
+    static_assert(sizeof...(P) == N);
+    return {};
+  }
+};
+
+template <typename T> struct type_identity {
+  using type = T;
+};
+
+template <typename... P>
+void foo(typename type_identity<A<P...>>::type a, P...) {}
+
+void foo() {
+  foo(B<0>());
+  foo(B<5>(), 1, 2, 3, 4, 5);
+}
+
+}

[llvm-ci]

LLVM Buildbot has detected a new failure on builder llvm-clang-aarch64-darwin running on doug-worker-4 while building clang at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/19444

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'Clang-Unit :: ./AllClangUnitTests/9/48' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/unittests/./AllClangUnitTests-Clang-Unit-86994-9-48.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=48 GTEST_SHARD_INDEX=9 /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/unittests/./AllClangUnitTests
--

Script:
--
/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/unittests/./AllClangUnitTests --gtest_filter=TimeProfilerTest.ConstantEvaluationC99
--
/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang/unittests/Support/TimeProfilerTest.cpp:349: Failure
Expected equality of these values:
  R"(
Frontend (test.c)
| ParseDeclarationOrFunctionDefinition (test.c:2:1)
| | isIntegerConstantExpr (<test.c:3:18>)
| | EvaluateKnownConstIntCheckOverflow (<test.c:3:18>)
| PerformPendingInstantiations
)"
    Which is: "\nFrontend (test.c)\n| ParseDeclarationOrFunctionDefinition (test.c:2:1)\n| | isIntegerConstantExpr (<test.c:3:18>)\n| | EvaluateKnownConstIntCheckOverflow (<test.c:3:18>)\n| PerformPendingInstantiations\n"
  buildTraceGraph(Json)
    Which is: "\nFrontend (test.c)\n| ParseDeclarationOrFunctionDefinition (test.c:2:1)\n| | isIntegerConstantExpr (<test.c:3:18>)\n| | EvaluateKnownConstIntCheckOverflow (<test.c:3:18>)\n| | PerformPendingInstantiations\n"
With diff:
@@ -4,3 +4,3 @@
 | | isIntegerConstantExpr (<test.c:3:18>)
 | | EvaluateKnownConstIntCheckOverflow (<test.c:3:18>)
-| PerformPendingInstantiations\n
+| | PerformPendingInstantiations\n



/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang/unittests/Support/TimeProfilerTest.cpp:349
Expected equality of these values:
  R"(
Frontend (test.c)
| ParseDeclarationOrFunctionDefinition (test.c:2:1)
| | isIntegerConstantExpr (<test.c:3:18>)
| | EvaluateKnownConstIntCheckOverflow (<test.c:3:18>)
| PerformPendingInstantiations
)"
    Which is: "\nFrontend (test.c)\n| ParseDeclarationOrFunctionDefinition (test.c:2:1)\n| | isIntegerConstantExpr (<test.c:3:18>)\n| | EvaluateKnownConstIntCheckOverflow (<test.c:3:18>)\n| PerformPendingInstantiations\n"
  buildTraceGraph(Json)
    Which is: "\nFrontend (test.c)\n| ParseDeclarationOrFunctionDefinition (test.c:2:1)\n| | isIntegerConstantExpr (<test.c:3:18>)\n| | EvaluateKnownConstIntCheckOverflow (<test.c:3:18>)\n| | PerformPendingInstantiations\n"
With diff:
@@ -4,3 +4,3 @@
 | | isIntegerConstantExpr (<test.c:3:18>)
 | | EvaluateKnownConstIntCheckOverflow (<test.c:3:18>)
-| PerformPendingInstantiations\n
+| | PerformPendingInstantiations\n

...