Fix template instantiation of UDLs

[aaron.ballman]

Summary

Previously, we would instantiate the UDL by marking the function as referenced and potentially binding to a temporary; this skipped transforming the call when the UDL was dependent on a template parameter.

Now, we defer all the work to instantiating the call expression for the UDL. This ensures that constant evaluation occurs at compile time rather than deferring until runtime.

Fixes Issue 54578.

[erichkeane]

clang/lib/Sema/TreeTransform.h

10516

I THINK you have to do this by doing getDerived().TransformCallExpr(E).

[cor3ntin]

LGTM.

clang/test/CodeGenCXX/cxx20-consteval-crash.cpp

28

I've used things like ghXXXX previously. for my own curiosity, is there a convention here?

[cor3ntin]

clang/lib/Sema/TreeTransform.h

10516

Oh yes, good point

[aaron.ballman]

clang/lib/Sema/TreeTransform.h

10516

Hmmm, don't we want this level of transformation to kick in, not the derived? e.g., another approach would be to remove the definition of this function entirely so that the recursive AST visitor calls TransformCallExpr() instead?

clang/test/CodeGenCXX/cxx20-consteval-crash.cpp

28

I don't know if we've organically gotten a convention here yet or not. I've been using Issue for mine, but don't have a strong preference either.

[erichkeane]

clang/lib/Sema/TreeTransform.h

10516

No, the idea is that TreeTransform is inherited by other 'instantiator' types. IF those types do something special for CallExpr, we want this to ALSO do the same thing as CallExpr.

I was also thinking about removing the function altogether. I just couldn't remember if TreeTransform did that right :) I'd suggest that if possible.

[aaron.ballman]

Update based on review feedback

[cor3ntin]

clang/lib/Sema/TreeTransform.h

10516

I tried to remove the overload entirely, clang was deeply unhappy :)

[aaron.ballman]

Thanks for the reviews! I've commit in ca844ab01c3f9410ceca967c09f809400950beae.

clang/docs/ReleaseNotes.rst

 -----------------------------
 
 - ...
 
 C++20 Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 - Diagnose consteval and constexpr issues that happen at namespace scope. This
   partially addresses `Issue 51593 <https://github.com/llvm/llvm-project/issues/51593>`_.
+- No longer attempt to evaluate a consteval UDL function call at runtime when
+  it is called through a template instantiation. This fixes
+  `Issue 54578 <https://github.com/llvm/llvm-project/issues/54578>`_.
 
 C++2b Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 - Implemented `P2128R6: Multidimensional subscript operator <https://wg21.link/P2128R6>`_.
 - Implemented `P0849R8: auto(x): decay-copy in the language <https://wg21.link/P0849R8>`_.
 - Implemented `P2242R3: Non-literal variables (and labels and gotos) in constexpr functions <https://wg21.link/P2242R3>`_.
 

clang/lib/Sema/TreeTransform.h

 ExprResult
 TreeTransform<Derived>::TransformCharacterLiteral(CharacterLiteral *E) {
   return E;
 }
 
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformUserDefinedLiteral(UserDefinedLiteral *E) {
-  if (FunctionDecl *FD = E->getDirectCallee())
-    SemaRef.MarkFunctionReferenced(E->getBeginLoc(), FD);
-  return SemaRef.MaybeBindToTemporary(E);
+  return getDerived().TransformCallExpr(E);
 }
 
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformGenericSelectionExpr(GenericSelectionExpr *E) {
   ExprResult ControllingExpr =
     getDerived().TransformExpr(E->getControllingExpr());
   if (ControllingExpr.isInvalid())

clang/test/CodeGenCXX/cxx20-consteval-crash.cpp

 void f() { g(); }
 
 // CHECK: define dso_local void @_ZN7PR514841fEv() #0 {
 // CHECK: entry:
 // CHECK-NOT: call i32 @_ZN7PR514841gEv()
 // CHECK:  ret void
 // CHECK: }
 }
+
+namespace Issue54578 {
+inline consteval unsigned char operator""_UC(const unsigned long long n) {
+  return static_cast<unsigned char>(n);
+}
+
+inline constexpr char f1(const auto octet) {
+  return 4_UC;
+}
+
+template <typename Ty>
+inline constexpr char f2(const Ty octet) {
+  return 4_UC;
+}
+
+int foo() {
+  return f1('a') + f2('a');
+}
+
+// Because the consteval functions are inline (implicitly as well as
+// explicitly), we need to defer the CHECK lines until this point to get the
+// order correct. We want to ensure there is no definition of the consteval
+// UDL function, and that the constexpr f1 and f2 functions both return a
+// constant value.
+
+// CHECK-NOT: define{{.*}} zeroext i8 @_ZN10Issue54578li3_UCEy
+// CHECK: define{{.*}} i32 @_ZN10Issue545783fooEv(
+// CHECK: define{{.*}} signext i8 @_ZN10Issue545782f1IcEEcT_(
+// CHECK: ret i8 4
+// CHECK: define{{.*}} signext i8 @_ZN10Issue545782f2IcEEcT_(
+// CHECK: ret i8 4
+}

clang/test/SemaCXX/cxx2a-consteval.cpp

struct T {
   Ty Val; // expected-note {{subobject declared here}}
   consteval T() {}
 };
 
 T<int> t; // expected-error {{call to consteval function 'NamespaceScopeConsteval::T<int>::T' is not a constant expression}} \
              expected-note {{subobject of type 'int' is not initialized}}
 
 } // namespace NamespaceScopeConsteval
+
+namespace Issue54578 {
+// We expect the user-defined literal to be resovled entirely at compile time
+// despite being instantiated through a template.
+inline consteval unsigned char operator""_UC(const unsigned long long n) {
+  return static_cast<unsigned char>(n);
+}
+
+inline constexpr char f1(const auto octet) {
+  return 4_UC;
+}
+
+template <typename Ty>
+inline constexpr char f2(const Ty octet) {
+  return 4_UC;
+}
+
+void test() {
+  static_assert(f1('a') == 4);
+  static_assert(f2('a') == 4);
+  constexpr int c = f1('a') + f2('a');
+  static_assert(c == 8);
+}
+}