Implement P1937 consteval in unevaluated contexts

[cor3ntin]

Summary

In an unevaluated contexts, consteval functions
should not be immediately evaluated.

[cor3ntin]

Note that I think this was partially implemented as part of https://reviews.llvm.org/D74130 - which has not progressed since October.
This PR implements P1937 only

[aaron.ballman]

clang/test/SemaCXX/cxx2a-consteval.cpp

603

608

612

Here's an interesting test case:

#include <typeinfo>

struct S {
  virtual void f();
};

struct D : S {
  void f() override;
};

consteval S *get_s() { return nullptr; }

void func() {
  (void)typeid(*get_s());
}

typeid still needs to evaluate its operand (due to the polymorphic return type of *get_s()), and so you should get a diagnostic about evaluating the side effects by calling get_s(). I think this then runs into https://eel.is/c++draft/expr.const#13.sentence-3 and we should diagnose?

[cor3ntin]

Fix formatting

[cor3ntin]

clang/test/SemaCXX/cxx2a-consteval.cpp

612

Not sure!
Also, in the context of this pr, the question is also whether decltype(typeid(*get_s())) should be ill-formed I think

[cor3ntin]

clang/test/SemaCXX/cxx2a-consteval.cpp

612

Actually, I'm reading the wording again and I really don't know anymore.
get_s() is a constant expression, right?
*get_s() is not, I think but is that relevant here

I played with a bunch of things in the code but the more I look at it the less I'm convinced an action is needed.

[aaron.ballman]

clang/test/SemaCXX/cxx2a-consteval.cpp

612

The changes to Sema::CheckForImmediateInvocation() to check for an unevaluated context and https://eel.is/c++draft/expr.const#13.sentence-3 that say an immediate invocation shall be a constant expression are what got me thinking about this code snippet in the first place. I was trying to decide whether isUnevaluatedContext() is correct or not because with typeid, it is potentially evaluated (so sometimes it's unevaluated).

Interestingly, everyone comes up with a different answer: https://godbolt.org/z/TqjGh1he6 and I don't (yet) know who is correct.

[cor3ntin]

clang/test/SemaCXX/cxx2a-consteval.cpp

612

@rsmith Can you enlighten us here?
My take is that get_s() is a constant expression and therefore an immediate invocation. independently of what *get_s() does but I'm not sure if that's a correct reading.

Thanks a lot!

[cor3ntin]

@rsmith Ping again! Thanks

[rsmith]

clang/test/SemaCXX/cxx2a-consteval.cpp

612

There are a few different cases here and I don't think any compiler is getting them all right.

struct S {
  void f();
};
struct T {
  virtual void f();
};

consteval S *null_s() { return nullptr; }
consteval S *make_s() { return new S; }
consteval T *null_t() { return nullptr; }
consteval T *make_s() { return new T; }

void func() {
  (void)typeid(*null_s()); // #1
  (void)typeid(*make_s()); // #2
  (void)typeid(*null_t()); // #3
  (void)typeid(*make_t()); // #4
}

Here, #3 and #4 pass an lvalue of polymorphic class type to typeid, so the arguments to those typeids are potentially evaluated. #1 and #2 pass an lvalue of non-polymorphic class type, so those arguments are unevaluated operands. So we have two immediate invocations: the null_t() call and the make_t() call.

Lines #1 and #2 are valid because there's no immediate invocation to check. (Clang and GCC get this wrong and reject #2.)
Line #3 is valid because the call to null_t() is a constant expression. (MSVC gets this wrong for reasons I don't understand.)
Line #4 is ill-formed because the call to make_t() is not a constant expression, because it returns a heap allocation.

The way we handle typeid in general is to parse its operand as an unevaluated operand, and then later TransformToPotentiallyEvaluated if we find it's a glvalue of or pointer to polymorphic class type. If you find the above testcase isn't handled correctly, you may need to make some changes in TransformToPotentiallyEvaluated to trigger the proper rebuilding. (You might need to force it to transform CallExprs that refer to consteval functions even if nothing within them have changed, for example.)

[cor3ntin]

• Add tests for typeid

All the tests suggested by Richard pass as expected without having to
modify the implementation of type id itself!

[cor3ntin]

clang/test/SemaCXX/cxx2a-consteval.cpp

612

Thanks, this was super useful!

I think clang gets everything right now - I added your scenarios as tests with the other typeid tests

[aaron.ballman]

clang/test/CXX/basic/basic.def.odr/p2-typeid.cpp

53–62

Cleaning up the test a bit to not use relative locations, should be NFC.

clang/test/SemaCXX/cxx2a-consteval.cpp

11

Looks like a bunch of unintended whitespace changes snuck in.

608–609

[cor3ntin]

Remove WS changes and cleanup tests

[cor3ntin]

Changing release version to clang 14 in cxx_status

[aaron.ballman]

LGTM!

[aaron.ballman]

I have commit on your behalf in 131b4620ee7847102479f399ce3e35a3c1cb5461, thank you for the fix!

[erichkeane]

clang/www/cxx_status.html

1106

I'm trying to evaluate our consteval support, and I am having trouble finding any unsuperceded part of P1073R3 that is not implemented in Clang14. Can you share what Delta you know of that has you considering this as 'partial'? Is it something in a different patch awaiting review?

[cor3ntin]

clang/www/cxx_status.html

1106

This thing https://reviews.llvm.org/D74130 is what I believe the last bit of missing functionality.
Note that I only implemented P1937 myself

[erichkeane]

clang/www/cxx_status.html

1106

Ah, thanks!

clang/lib/Sema/SemaExpr.cpp

if (BO->getOpcode() == BO_Assign) {
       auto &LHSs = ExprEvalContexts.back().VolatileAssignmentLHSs;
       LHSs.erase(std::remove(LHSs.begin(), LHSs.end(), BO->getLHS()),
                  LHSs.end());
     }
   }
 }
 
 ExprResult Sema::CheckForImmediateInvocation(ExprResult E, FunctionDecl *Decl) {
-  if (!E.isUsable() || !Decl || !Decl->isConsteval() || isConstantEvaluated() ||
+  if (isUnevaluatedContext() || !E.isUsable() || !Decl ||
+      !Decl->isConsteval() || isConstantEvaluated() ||
       RebuildingImmediateInvocation)
     return E;
 
   /// Opportunistically remove the callee from ReferencesToConsteval if we can.
   /// It's OK if this fails; we'll also remove this in
   /// HandleImmediateInvocations, but catching it here allows us to avoid
   /// walking the AST looking for it in simple cases.
   if (auto *Call = dyn_cast<CallExpr>(E.get()->IgnoreImplicit()))
void Sema::MarkDeclRefReferenced(DeclRefExpr *E, const Expr *Base) {
   // if it's a qualified reference.
   bool OdrUse = true;
   if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(E->getDecl()))
     if (Method->isVirtual() &&
         !Method->getDevirtualizedMethod(Base, getLangOpts().AppleKext))
       OdrUse = false;
 
   if (auto *FD = dyn_cast<FunctionDecl>(E->getDecl()))
-    if (!isConstantEvaluated() && FD->isConsteval() &&
-        !RebuildingImmediateInvocation)
+    if (!isUnevaluatedContext() && !isConstantEvaluated() &&
+        FD->isConsteval() && !RebuildingImmediateInvocation)
       ExprEvalContexts.back().ReferenceToConsteval.insert(E);
   MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse,
                      RefsMinusAssignments);
 }
 
 /// Perform reference-marking and odr-use handling for a MemberExpr.
 void Sema::MarkMemberReferenced(MemberExpr *E) {
   // C++11 [basic.def.odr]p2:

clang/test/CXX/basic/basic.def.odr/p2-typeid.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
 
 // C++ [basic.def.odr]p2:
 //   An expression is potentially evaluated unless it [...] is the
 //   operand of the typeid operator and the expression does not
 //   designate an lvalue of polymorphic class type.
 
 // FIXME: This should really include <typeinfo>, but we don't have that yet.
 namespace std {
   class type_info;
 }
 
 struct Poly {
   virtual ~Poly();
 };
 
 struct NonPoly { };
 
-template<typename T, typename Result = T> 
+template<typename T, typename Result = T>
 struct X {
   Result f(T t) { return t + t; } // expected-error{{invalid operands}}
 
   void g(T t) {
     (void)typeid(f(t)); // expected-note{{here}}
   }
 };
 
 void test(X<Poly> xp, X<Poly, Poly&> xpr, X<NonPoly> xnp, X<NonPoly, NonPoly&> xnpr) {
   // These are okay (although GCC and EDG get them wrong).
   xp.g(Poly());
   xnp.g(NonPoly());
   xnpr.g(NonPoly());
 
   // Triggers an error (as it should);
   xpr.g(Poly()); // expected-note{{instantiation of member function}}
 }
+
+#if __cplusplus >= 202002L
+
+namespace unevaluated {
+
+struct S {
+  void f();
+};
+struct T {
+  virtual void f();
+};
+
+consteval S *null_s() { return nullptr; }
+consteval S *make_s() { return new S; }
+consteval T *null_t() { return nullptr; }
+consteval T *make_t() { return new T; } // #alloc
+
+void func() {
+  (void)typeid(*null_s());
+  (void)typeid(*make_s());
+  (void)typeid(*null_t()); // expected-warning {{expression with side effects will be evaluated despite being used as an operand to 'typeid'}}
+  (void)typeid(*make_t()); // expected-error {{call to consteval function 'unevaluated::make_t' is not a constant expression}} \
+                              expected-note {{pointer to heap-allocated object is not a constant expression}} \
+                              expected-note@#alloc {{heap allocation performed here}} \
+                              expected-warning {{expression with side effects will be evaluated despite being used as an operand to 'typeid'}}
+}
+
+} // namespace unevaluated
+
+#endif

clang/test/SemaCXX/cxx2a-consteval.cpp

 // RUN: %clang_cc1 -std=c++2a -emit-llvm-only -Wno-unused-value %s -verify
 
 typedef __SIZE_TYPE__ size_t;
 
 namespace basic_sema {
 
 consteval int f1(int i) {
   return i;
 }
 
 consteval constexpr int f2(int i) { 
   //expected-error@-1 {{cannot combine}}
   return i;
 }
 
 constexpr auto l_eval = [](int i) consteval {
 // expected-note@-1+ {{declared here}}
 
   return i;
void test() {
   { Copy* c; c = new Copy(std::move(C)); }
   { Copy* c; c = new Copy(std::move(Copy(&f_eval))); }// expected-error {{is not a constant expression}} expected-note {{to a consteval}}
   { Copy* c; c = new Copy(to_lvalue_ref(Copy(&f_eval))); }// expected-error {{is not a constant expression}} expected-note {{to a consteval}}
   { Copy* c; c = new Copy(to_lvalue_ref(std::move(C))); }
   { Copy* c; c = new Copy(to_lvalue_ref(std::move(Copy(&f_eval)))); }// expected-error {{is not a constant expression}} expected-note {{to a consteval}}
 }
 
 } // namespace special_ctor
+
+namespace unevaluated {
+
+template <typename T, typename U> struct is_same { static const bool value = false; };
+template <typename T> struct is_same<T, T> { static const bool value = true; };
+
+long f(); // expected-note {{declared here}}
+auto consteval g(auto a) {
+  return a;
+}
+
+auto e = g(f()); // expected-error {{is not a constant expression}}
+                 // expected-note@-1 {{non-constexpr function 'f' cannot be used in a constant expression}}
+
+using T = decltype(g(f()));
+static_assert(is_same<long, T>::value);
+
+} // namespace unevaluated

clang/www/cxx_status.html

</tr>
     <tr>
       <td><tt>char8_t</tt></td>
       <td><a href="https://wg21.link/p0482r6">P0482R6</a></td>
       <td class="full" align="center">Clang 7 <a href="#p0482">(11)</a></td>
     </tr>
     <tr>
       <td rowspan=2>Immediate functions (<tt>consteval</tt>)</td>
       <td><a href="https://wg21.link/p1073r3">P1073R3</a></td>
-      <td rowspan=2 class="none" align="center">No</td>
+      <td class="partial" align="center">Partial</td>
     </tr>
       <tr> <!-- from Prague -->
         <td><a href="https://wg21.link/p1937r2">P1937R2</a></td>
+        <td class="unreleased" align="center">Clang 14</td>
       </tr>
     <tr>
       <td><tt>std::is_constant_evaluated</tt></td>
       <td><a href="https://wg21.link/p0595r2">P0595R2</a></td>
       <td class="full" align="center">Clang 9</td>
     </tr>
     <tr>
       <td>Nested inline namespaces</td>