[PATCH] D55500: [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D55500#1347493 <https://reviews.llvm.org/D55500#1347493>, @EricWF wrote:

> @rsmith, what sorts of additional tests are needed?


Let's see...

- test that we use the constant initializer for suitably-constant-initialized locals even if we can't constant-fold the reference to them:

  bool f(int k) {
    const bool n = is_constant_evaluated();
    const bool *volatile p = n;
    return *p;
  }



- test that we suitably handle local references initialized by constant expressions (and local `constexpr` variables), and non-`const` globals
- test that `__builtin_is_constant_evaluated()` returns false in contexts that IR generation might constant-fold (eg, the operand of a non-`constexpr` `if` or the left-hand side of a `?:` expression)
- ensure that the right value is returned in all the cases that are syntactically //constant-expression//s in the C++ grammar:
  - second or subsequent array bound in a //new-expression//
  - `case` labels
  - operand of `static_assert`
  - array bounds (but be careful around the treatment of VLAs! for them we want the "try evaluating it as a constant expression and fall back to treating it as a non-constant expression" rule that we have for the initializers of globals)
  - values of enumerators
  - operand of `alignas`
  - width of a bit-field
  - template arguments (but bizarrely not default template arguments! I think that's a bug and I'll bring it up on the core reflector.)
  - operand of `noexcept` (and eventually `explicit` -- please write a test for the latter with a FIXME to update the expectations, so we don't forget)



================
Comment at: include/clang/Basic/Builtins.def:758
 
+// Random C++ builtins.
+LANGBUILTIN(__builtin_is_constant_evaluated, "b", "ncu", CXX_LANG)
----------------
This should be grouped with `__builtin_launder`, though maybe moving that one here makes more sense than grouping them both under "GCC builtins" since they're directly implementing C++ stdlib functions.


================
Comment at: include/clang/Basic/Builtins.def:759
+// Random C++ builtins.
+LANGBUILTIN(__builtin_is_constant_evaluated, "b", "ncu", CXX_LANG)
+
----------------
bruno wrote:
> EricWF wrote:
> > EricWF wrote:
> > > bruno wrote:
> > > > Name bikeshedding : perhaps the builtin name could be detached from the std:: name? Suggestion: `__builtin_in_constant_evaluation_context`
> > > I'm not sure detaching it from the `std::` name is desirable. Most importantly it should match w/e GCC does/decides to do.
> > > 
> > > But if it is, we should name in deference to the standardese it implements. Specifically weither an expression or conversion is //manifestly constant-evaluated// [[expr.const](http://eel.is/c++draft/expr.const#11)]p11.
> > > 
> > > Therefore I proffer  `__builtin_is_manifestly_constant_evaluated()` or `__builtin_is_being_manifestly_constant_evaluated()`.
> > > 
> > > 
> > Actually, GCC has `__builtin_is_constant_evaluated` so we should use that name too.
> Agreed!
I don't think the "u" flag has any meaning here (there are no arguments, so no unevaluated arguments), and it would reduce the complexity of this declaration if you remove it.

I don't think the "c" flag is formally correct -- a program can contain two calls to `__builtin_is_constant_evaluated()` that return different values (even "U" appears to be incorrect). It would, for example, be reasonable for Clang to warn on:

```
const bool b = pure_fn();
return b == pure_fn();
```

... on the basis that the comparison must always evaluate to `true`. But such a warning would be incorrect for `__builtin_is_constant_evaluated`, so it's not a pure function.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55500/new/

https://reviews.llvm.org/D55500