[AST] Ignore implicit nodes in CastExpr::getConversionFunction

[kimgr]

Summary

Under -std=c++20 with use of consteval, the subexpression hierarchy sometimes
contains ConstantExpr nodes that getConversionFunction did not expect.

CastExpr::getConversionFunction is very rarely used, so this was difficult to
trigger in the compiler. It is possible to reproduce using -ast-dump=json, which
triggers an assertion for inputs with consteval implicit casts.

In AST-based tools, however, it surfaces quite easily if they try to inspect the
conversion function of a visited CastExpr.

Add two new testcases, both for implicit constructor conversions and
user-defined conversions with consteval.

Depends on D117390

[kimgr]

Oh yeah, this closes bug https://github.com/llvm/llvm-project/issues/53044. Do I mention that in the commit message/patch summary?

[davrec]

Looks good, thanks for fixing this!

[kimgr]

clang/unittests/Tooling/CastExprTest.cpp

111

Oops, spurious semicolon here. Will post a new version once I've tested it.

[kimgr]

Fix spurious semicolon

[kimgr]

• Update broken test case
• Rebase on latest main (8eb74626f)
• Build and run ninja check-clang

I don't have commit access, so I would appreciate help landing. Let me know if there's anything else I can do before.

[kimgr]

@rsmith Weekly ping

[kimgr]

@rsmith Gentle ping

[aaron.ballman]

I think the changes here look reasonable, though I think it can be simplified a bit.

clang/lib/AST/Expr.cpp

1946–1947

IgnoreImplicit() calls IgnoreImplicitSingleStep() eventually, and that call does the same work as skipImplicitTemporary(), so I think the end result here should be the same.

[davrec]

clang/lib/AST/Expr.cpp

1946–1947

As I look at this a second time, I just realized...calling IgnoreImplicit here mean that the loop only ever runs one iteration, since IgnoreImplicit presumably skips over ImplicitCastExprs. While I liked how Kim did revision initially because it seemed to handle the constructor conversions similarly to their handling of getSubExprAsWritten() above, now I think something different is needed here.

Proposed alternative:
Right now skipIimplicitTemporary does what IgnoreImplicit does *except* skip over a) ImplicitCastExprs and b) FullExprs (= ConstantExprs or ExprWithCleanups).

Kim has identified that we need to skip over at least ConstantExprs at least in this case (i.e. the most conservative possible fix would be to skip over ConstantExprs just before the cast in line 1950).

But perhaps the better solution, to forestall future bugs, is to skip over FullExprs in skipImplicitTemporary, so that it skips over everything IgnoreImplicit does except ImplicitCastExprs. (And, some documentation should be added to skipImplicitTemporary to that effect, to aid future maintenance.)

I can't see offhand how the other uses of skipImplicitTemporary would be negatively affected by additionally skipping over FullExprs.

Aaron what do you think? Kim can you verify this alternative would also solve the problem without breaking any tests?

[kimgr]

@aaron.ballman @davrec Thank you both!

I started with @davrec's suggestion from https://github.com/llvm/llvm-project/issues/53044#issuecomment-1006894536, and while it fixed the problem it broke several other consteval lit tests.

It could be that those test breaks are benign, but they startled me a little, so I decided to narrow the fix.

I'll try and rebase on latest and experiment with a few different approaches and report back.

[kimgr]

Alright, with this diff:

diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index d502b3f1e93e..1716ac0be7ef 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1908,6 +1908,9 @@ namespace {
     if (auto *Binder = dyn_cast<CXXBindTemporaryExpr>(E))
       E = Binder->getSubExpr();
 
+    if (auto *Full = dyn_cast<FullExpr>(E))
+      E = Full->getSubExpr();
+
     return E;
   }
 }
@@ -1944,7 +1947,6 @@ NamedDecl *CastExpr::getConversionFunction() const {
 
   for (const CastExpr *E = this; E; E = dyn_cast<ImplicitCastExpr>(SubExpr)) {
     SubExpr = skipImplicitTemporary(E->getSubExpr());
-    SubExpr = SubExpr->IgnoreImplicit();
 
     if (E->getCastKind() == CK_ConstructorConversion)
       return cast<CXXConstructExpr>(SubExpr)->getConstructor();

I get these test failures for ninja check-clang:

FAIL: Clang :: SemaCXX/cxx2a-consteval.cpp (71 of 970)
******************** TEST 'Clang :: SemaCXX/cxx2a-consteval.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/kimgr/code/llvm-project/out/main/bin/clang -cc1 -internal-isystem /home/kimgr/code/llvm-project/out/main/lib/clang/15.0.0/include -nostdsysteminc -std=c++2a -emit-llvm-only -Wno-unused-value /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp -verify
--
Exit Code: 1

Command Output (stderr):
--
error: 'error' diagnostics seen but not expected: 
  File /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp Line 362: call to consteval function 'alloc::to_lvalue_ref' is not a constant expression
  File /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp Line 364: call to consteval function 'alloc::A::ret_a' is not a constant expression
  File /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp Line 370: call to consteval function 'alloc::A::ret_a' is not a constant expression
  File /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp Line 371: call to consteval function 'alloc::A::ret_a' is not a constant expression
  File /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp Line 377: call to consteval function 'alloc::A::ret_a' is not a constant expression
error: 'note' diagnostics seen but not expected: 
  File /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp Line 362: reference to temporary is not a constant expression
  File /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp Line 364: pointer to heap-allocated object is not a constant expression
  File /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp Line 370: pointer to heap-allocated object is not a constant expression
  File /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp Line 371: pointer to heap-allocated object is not a constant expression
  File /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp Line 377: pointer to heap-allocated object is not a constant expression
10 errors generated.

I'm going to dive in and see if I can judge who is right, tests or code.

[kimgr]

Here's the diff between my patch (main) and handling FullExpr in skipImplicitTemporary as in the diff I posted above (patch.txt):

--- main.txt	2022-02-02 20:37:21.619534225 +0100
+++ patch.txt	2022-02-02 20:34:17.016949227 +0100
@@ -192,6 +192,13 @@
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:360:25: note: temporary created here
   { A k = to_lvalue_ref(A()); } // expected-error {{is not a constant expression}}
                         ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:362:25: error: call to consteval function 'alloc::A::ret_a' is not a constant expression
+  { A k = to_lvalue_ref(A().ret_a()); } // expected-error {{is not a constant expression}}
+                        ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:362:25: note: pointer to heap-allocated object is not a constant expression
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:334:12: note: heap allocation performed here
+  int* p = new int(42);
+           ^
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:362:11: error: call to consteval function 'alloc::to_lvalue_ref' is not a constant expression
   { A k = to_lvalue_ref(A().ret_a()); } // expected-error {{is not a constant expression}}
           ^
@@ -199,6 +206,27 @@
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:362:25: note: temporary created here
   { A k = to_lvalue_ref(A().ret_a()); } // expected-error {{is not a constant expression}}
                         ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:364:13: error: call to consteval function 'alloc::A::ret_a' is not a constant expression
+  { int k = A().ret_a().ret_i(); }
+            ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:364:13: note: pointer to heap-allocated object is not a constant expression
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:334:12: note: heap allocation performed here
+  int* p = new int(42);
+           ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:370:25: error: call to consteval function 'alloc::A::ret_a' is not a constant expression
+  { int k = const_a_ref(A().ret_a()); }
+                        ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:370:25: note: pointer to heap-allocated object is not a constant expression
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:334:12: note: heap allocation performed here
+  int* p = new int(42);
+           ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:371:39: error: call to consteval function 'alloc::A::ret_a' is not a constant expression
+  { int k = const_a_ref(to_lvalue_ref(A().ret_a())); }
+                                      ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:371:39: note: pointer to heap-allocated object is not a constant expression
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:334:12: note: heap allocation performed here
+  int* p = new int(42);
+           ^
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:375:14: error: call to consteval function 'alloc::A::ret_a' is not a constant expression
   { int k = (A().ret_a(), A().ret_i()); }// expected-error {{is not a constant expression}}
              ^
@@ -206,6 +234,13 @@
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:334:12: note: heap allocation performed here
   int* p = new int(42);
            ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:377:26: error: call to consteval function 'alloc::A::ret_a' is not a constant expression
+  { int k = (const_a_ref(A().ret_a()), A().ret_i()); }
+                         ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:377:26: note: pointer to heap-allocated object is not a constant expression
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:334:12: note: heap allocation performed here
+  int* p = new int(42);
+           ^
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:400:7: error: call to consteval function 'self_referencing::f' is not a constant expression
   s = f(0); // expected-error {{is not a constant expression}}
       ^
@@ -454,9 +489,9 @@
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:548:15: note: declared here
 consteval int f_eval() { // expected-note+ {{declared here}}
               ^
-/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:574:25: error: cannot take address of consteval function 'f_eval' outside of an immediate invocation
-  { Copy c = Copy(Copy(&f_eval)); }// expected-error {{cannot take address of consteval}}
-                        ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:567:19: error: cannot take address of consteval function 'f_eval' outside of an immediate invocation
+  { Copy c((Copy(&f_eval))); }// expected-error {{cannot take address of consteval}}
+                  ^
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:548:15: note: declared here
 consteval int f_eval() { // expected-note+ {{declared here}}
               ^
@@ -466,9 +501,9 @@
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:548:15: note: declared here
 consteval int f_eval() { // expected-note+ {{declared here}}
               ^
-/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:567:19: error: cannot take address of consteval function 'f_eval' outside of an immediate invocation
-  { Copy c((Copy(&f_eval))); }// expected-error {{cannot take address of consteval}}
-                  ^
+/home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:574:25: error: cannot take address of consteval function 'f_eval' outside of an immediate invocation
+  { Copy c = Copy(Copy(&f_eval)); }// expected-error {{cannot take address of consteval}}
+                        ^
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:548:15: note: declared here
 consteval int f_eval() { // expected-note+ {{declared here}}
               ^
@@ -501,4 +536,4 @@
     a() + d(); // expected-error {{call to consteval function 'PR48235::A::a' is not a constant expression}} \
     ^
 /home/kimgr/code/llvm-project/clang/test/SemaCXX/cxx2a-consteval.cpp:661:5: note: implicit use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function
-84 errors generated.
+89 errors generated.

It looks to my untrained eye as if these additional diagnostics might be correct. But I'm not sure. Help, please?

[davrec]

clang/lib/AST/Expr.cpp

1949–1950

I think the errors prove we should fall back to the most conservative possible fix: remove all the other changes and just change these 2 lines to:

if (E->getCastKind() == CK_ConstructorConversion) {
  if (auto *CE = dyn_cast<ConstantExpr>(SubExpr)
    SubExpr = CE->getSubExpr();
  return cast<CXXConstructExpr>(SubExpr)->getConstructor();
}

I'm can't quite remember but I believe that would solve the bug as narrowly as possible. @kimgr can you give it a try if and see if it avoids the errors?
(If it doesn't, I'm stumped...)

[kimgr]

clang/lib/AST/Expr.cpp

1949–1950

Yes, it does. I needed to add the same ConstantExpr skipping to the user-defined conversion handling below, but once those two were in place the new unittests succeed and the existing lit tests also.

It does feel a little awkward, though... I wish I had a clearer mental model of how the implicit-skipping is supposed to work. My intuition is telling me skipImplicitTemporary should probably disappear and we should use the IgnoreExpr.h facilities directly instead, but it's all a little fuzzy to me at this point.

I'll run the full check-clang suite overnight with this alternative patch, I have no reason to think there will be problems, but I'll make sure in the morning.

Thanks!

[kimgr]

clang/lib/AST/Expr.cpp

1949–1950

I can confirm that full check-clang also works with the narrower fix.

[kimgr]

I have now convinced myself that including FullExpr in skipImplicitTemporary gives an improvement in consteval diagnostics. But I'm still not sure why. Motivating example, derived from cxx2a-consteval.cpp:

struct A {
  int *p = new int(42);
  consteval A ret_a() const {
      return A{};
  }
};

consteval const A &to_lvalue_ref(const A &&a) {
  return a;
}

void test() {
  constexpr A a{nullptr};

  // error: call to consteval function 'A::ret_a' is not a constant expression
  // { (void)a.ret_a(); }

  // error: call to consteval function 'to_lvalue_ref' is not a constant expression
  // { (void)to_lvalue_ref(A{}); }

  // error: call to consteval function 'to_lvalue_ref' is not a constant expression
  // but should probably also raise
  // error: call to consteval function 'A::ret_a' is not a constant expression
  { (void)to_lvalue_ref(a.ret_a()); }
}

It's interesting to experiment with these one by one

• A::ret_a returns a new A, whose constructor does heap allocation; no consteval possible
• to_lvalue_ref attempts to return a reference to a temporary; no consteval possible

Composing the two as in the last example, it seems to me, should print both diagnostics. Mainline Clang doesn't, but changing skipImplicitTemporary to also skip FullExprs does (also allowing removal of all IgnoreImplicit from getSubExprAsWritten and getConversionFunction).

It seems intuitively right to me. I'm just a little peeved that I can't figure out the connection between the diagnostic emission and skipImplicitTemporary.

clang/lib/AST/Expr.cpp

1946–1947

@aaron.ballman Just removing the skipImplicitTemporary line is probably not going to work, since that's the first time SubExpr is assigned a

[aaron.ballman]

In D117391#3299483, @kimgr wrote:

I have now convinced myself that including FullExpr in skipImplicitTemporary gives an improvement in consteval diagnostics. But I'm still not sure why. Motivating example, derived from cxx2a-consteval.cpp:

struct A {
  int *p = new int(42);
  consteval A ret_a() const {
      return A{};
  }
};

consteval const A &to_lvalue_ref(const A &&a) {
  return a;
}

void test() {
  constexpr A a{nullptr};

  // error: call to consteval function 'A::ret_a' is not a constant expression
  // { (void)a.ret_a(); }

  // error: call to consteval function 'to_lvalue_ref' is not a constant expression
  // { (void)to_lvalue_ref(A{}); }

  // error: call to consteval function 'to_lvalue_ref' is not a constant expression
  // but should probably also raise
  // error: call to consteval function 'A::ret_a' is not a constant expression
  { (void)to_lvalue_ref(a.ret_a()); }
}

It's interesting to experiment with these one by one

Agreed. This stuff is about as clear as mud given that the rules change in every version of C++ and no two compilers implement the same constant expression evaluation rules at this point.

• A::ret_a returns a new A, whose constructor does heap allocation; no consteval possible

Agreed, though there are cases where it could be possible: http://eel.is/c++draft/expr.const#5.19.

• to_lvalue_ref attempts to return a reference to a temporary; no consteval possible

Hmm, is that still the case? http://eel.is/c++draft/expr.const#4.7

Composing the two as in the last example, it seems to me, should print both diagnostics. Mainline Clang doesn't, but changing skipImplicitTemporary to also skip FullExprs does (also allowing removal of all IgnoreImplicit from getSubExprAsWritten and getConversionFunction).

It seems intuitively right to me. I'm just a little peeved that I can't figure out the connection between the diagnostic emission and skipImplicitTemporary.

Assuming that the to_lvalue_ref(A{}) case should diagnose, then yes, I agree, I think the ret_a() portion should as well.

clang/lib/AST/Expr.cpp

1946–1947

Aaron what do you think? Kim can you verify this alternative would also solve the problem without breaking any tests?

I think that's a good idea to explore. I hadn't spotted the FullExpr difference when I was looking at it before.

1949–1950

My intuition is telling me skipImplicitTemporary should probably disappear and we should use the IgnoreExpr.h facilities directly instead, but it's all a little fuzzy to me at this point.

This matches the direction I think we should be heading. If we need to add a new facility to IgnoreExpr.h, that's also fine, but comments describing how it differs from the existing facilities would be critical.

[kimgr]

@aaron.ballman Thanks for the valuable feedback!

Assuming that the to_lvalue_ref(A{}) case should diagnose, then yes, I agree, I think the ret_a() portion should as well.

My primary focus is not to make sure Clang follows the standard closely for consteval, but rather to make sure it's self-consistent and doesn't provoke undefined behavior for consteval inputs.

So another way to think about it is that to_lvalue_ref and ret_a might both diagnose, or not, but absence of diagnostics should not be a result of misinterpreting the AST the way we do now.

I'm preparing a different patch to do the short-circuiting in skipImplicitTemporary, so we can more easily compare and contrast.

[aaron.ballman]

In D117391#3301696, @kimgr wrote:

@aaron.ballman Thanks for the valuable feedback!

Assuming that the to_lvalue_ref(A{}) case should diagnose, then yes, I agree, I think the ret_a() portion should as well.

My primary focus is not to make sure Clang follows the standard closely for consteval, but rather to make sure it's self-consistent and doesn't provoke undefined behavior for consteval inputs.

Agreed, but if self-consistency takes us further away from standards conformance, then we've got other problems to solve (and potentially need to solve those first).

So another way to think about it is that to_lvalue_ref and ret_a might both diagnose, or not, but absence of diagnostics should not be a result of misinterpreting the AST the way we do now.

Yup!

I'm preparing a different patch to do the short-circuiting in skipImplicitTemporary, so we can more easily compare and contrast.

Thanks!

[kimgr]

Now that D119477 has landed, this suggested change is obsolete. Closing.

clang/lib/AST/Expr.cpp

Expr *CastExpr::getSubExprAsWritten() {
 
   return const_cast<Expr*>(SubExpr);
 }
 
 NamedDecl *CastExpr::getConversionFunction() const {
   const Expr *SubExpr = nullptr;
 
   for (const CastExpr *E = this; E; E = dyn_cast<ImplicitCastExpr>(SubExpr)) {
     SubExpr = skipImplicitTemporary(E->getSubExpr());
+    SubExpr = SubExpr->IgnoreImplicit();
 
     if (E->getCastKind() == CK_ConstructorConversion)
       return cast<CXXConstructExpr>(SubExpr)->getConstructor();
 
     if (E->getCastKind() == CK_UserDefinedConversion) {
       if (auto *MCE = dyn_cast<CXXMemberCallExpr>(SubExpr))
         return MCE->getMethodDecl();
     }
   }
 
   return nullptr;

clang/unittests/Tooling/CastExprTest.cpp

 //===- unittest/Tooling/CastExprTest.cpp ----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "TestVisitor.h"
 
 using namespace clang;
 
 namespace {
 
 struct CastExprVisitor : TestVisitor<CastExprVisitor> {
   std::function<void(ExplicitCastExpr *)> OnExplicitCast;
+  std::function<void(CastExpr *)> OnCast;
 
   bool VisitExplicitCastExpr(ExplicitCastExpr *Expr) {
     if (OnExplicitCast)
       OnExplicitCast(Expr);
     return true;
   }
+
+  bool VisitCastExpr(CastExpr *Expr) {
+    if (OnCast)
+      OnCast(Expr);
+    return true;
+  }
 };
 
 TEST(CastExprTest, GetSubExprAsWrittenThroughMaterializedTemporary) {
   CastExprVisitor Visitor;
   Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
     auto Sub = Expr->getSubExprAsWritten();
     EXPECT_TRUE(isa<DeclRefExpr>(Sub))
         << "Expected DeclRefExpr, but saw " << Sub->getStmtClassName();
Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
     EXPECT_TRUE(isa<IntegerLiteral>(Sub))
         << "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
   };
   Visitor.runOver("struct S { consteval S(int) {} };\n"
                   "S f() { return S(0); }\n",
                   CastExprVisitor::Lang_CXX2a);
 }
 
+// Verify that getConversionFunction looks through a ConstantExpr for implicit
+// constructor conversions (https://github.com/llvm/llvm-project/issues/53044):
+//
+// `-ImplicitCastExpr 'S' <ConstructorConversion>
+// `-ConstantExpr 'S'
+//   |-value: Struct
+//   `-CXXConstructExpr 'S' 'void (const char *)'
+//     `-ImplicitCastExpr 'const char *' <ArrayToPointerDecay>
+//       `-StringLiteral 'const char [7]' lvalue "foobar"
+TEST(CastExprTest, GetCtorConversionFunctionThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnCast = [](CastExpr *Expr) {
+    if (Expr->getCastKind() == CK_ConstructorConversion) {
+      auto *Conv = Expr->getConversionFunction();
+      EXPECT_TRUE(isa<CXXConstructorDecl>(Conv))
+          << "Expected CXXConstructorDecl, but saw " << Conv->getDeclKindName();
+    }
+  };
+  Visitor.runOver("struct X { consteval X(const char *) {} };\n"
+                  "void f() { X x = \"foobar\"; }\n",
+                  CastExprVisitor::Lang_CXX2a);
+}
+
+// Verify that getConversionFunction looks through a ConstantExpr for implicit
+// user-defined conversions.
+//
+// `-ImplicitCastExpr 'const char *' <UserDefinedConversion>
+//   `-ConstantExpr 'const char *'
+//     |-value: LValue
+//     `-CXXMemberCallExpr 'const char *'
+//       `-MemberExpr '<bound member function type>' .operator const char *
+//         `-DeclRefExpr 'const X' lvalue Var 'x' 'const X'
+TEST(CastExprTest, GetUserDefinedConversionFunctionThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnCast = [](CastExpr *Expr) {
+    if (Expr->getCastKind() == CK_UserDefinedConversion) {
+      auto *Conv = Expr->getConversionFunction();
+      EXPECT_TRUE(isa<CXXMethodDecl>(Conv))
+          << "Expected CXXMethodDecl, but saw " << Conv->getDeclKindName();
+    }
+  };
+  Visitor.runOver("struct X {\n"
+                  "  consteval operator const char *() const {\n"
+                  "    return nullptr;\n"
+                  "  }\n"
+                  "};\n"
+                  "const char *f() {\n"
+                  "  constexpr X x;\n"
+                  "  return x;\n"
+                  "}\n",
+                  CastExprVisitor::Lang_CXX2a);
+}
+
 } // namespace