[clang] [Clang] Instantiate functions from constant evaluation. (PR #205557)

[Daniel M. Katz via cfe-commits]

katzdm wrote:

# Okay, _so_...

First thing's first; locally, I've pulled and merged latest `main` - no (apparent) problems there. ✅ 

Second thing's second; yes, all of the below was written with my fingers. That doesn't quite guarantee that it isn't slop, but at least it isn't AI slop (possibly just @katzdm slop).

A lot of discussion has (understandably) transpired around this PR, so I'm going to try to 1) summarize known concerns and 2) describe outstanding work that I intend to do before this lands.

## When should side-effects be permitted?
I talked this through with @zygoloid recently - my feeling is that side-effects should be permitted during the evaluation of an expression that either
1. is manifestly constant-evaluated (e.g., initializers of constexpr variables, constant template arguments, static assertion operands, etc) or
2. must be evaluated (as mandated by language rules) in order to determine whether it is manifestly constant-evaluated (e.g., initializers of variables with static storage duration, immediate invocations).

I think that whereas (1) seems well accepted, (2) is a bit controversial. I'll point out that this appears to follow GCC's and MSVC's approach; see https://godbolt.org/z/z1hKz5bWr: If side-effects were disallowed during such "checking for constant-ness", then `g()` would fail to be a constant expression, `v` would not be usable in constant expressions, and the initializer of `z` would be ill-formed. It would be lovely to see the Standard better spell this out, and I do hope to work on that.

The other category of expressions which seems fine to produce side-effects from is, of course, non-standard constructs (e.g., `[[annotate(...)]]`, `__attribute__((enable_if(....)))`, `__builtin_matrix_column_major_load`). We should consider allowing side-effects from these extensions on a case-by-case basis, but they have nothing to do with language-mandated constant evaluation, so I think they're outside the scope of this PR (note for posterity: call-sites that would be updated are [here](https://github.com/llvm/llvm-project/blob/b0f50174f52d02eb68a1194ed93ec89507ef2fe1/clang/lib/Sema/SemaAttr.cpp#L575) and [here](https://github.com/llvm/llvm-project/blob/b0f50174f52d02eb68a1194ed93ec89507ef2fe1/clang/lib/Sema/SemaChecking.cpp#L17354)).

### Narrowing conversions

The above does not quite cover all interesting cases: Notably, there is the case of **narrowing conversions in braces and initializer-lists**. This is one of very few super odd corners where the language requires constant evaluation of an expression that is known not to be manifestly constant-evaluated.

Per the mental model above for which "instantiation from constant evaluation" is only permitted during the evaluation of a manifestly constant-evaluated expression, or during the determination of whether an expression is manifestly constant-evaluated, I would like to, for now, continue to disallow instantiation during narrowing conversion checks. Going forward, I hope to write a paper to reclassify such expressions as manifestly constant-evaluated - if WG21 accepts this direction, then we can allow instantiation from these evaluations at that time.

If we accept this direction, then I think it falls out from there that we should keep `EvalInfo::InConstantContext` around for now, since there are (for now) some language-mandated constant evaluations for which side-effects are disallowed (and, therefore, the presence of a `SemaProxy` is an imperfect representation of when we are in a language-mandated constant context).

## Remaining cases raised by @jyknight 

@jyknight raised several concerns further up-thread. I spoke to some of them (e.g., non-standard attributes) above, but let me speak to the rest of them (to the best of my ability) here:

- **Non-constexpr globals**: If we're following the framework above (i.e., side-effects can be produced while checking whether an expression is manifestly constant-evaluated), then **this is a bug** which I want to fix in this PR. Here is a litmus test, which should exit with status 1: https://godbolt.org/z/cT4oerfsh
- **"friend" declaration with explicit template-args**: I'd have to see an example for this one - @jyknight, any chance you could help to produce one?
- **issues with `immediate function {} used before it is defined` related to `consteval` functions**: I'd have to see an example here, but the error message makes me think that this could be a case where a not-yet-defined `consteval` function is called from within a `constexpr` function (i.e., maybe something like this? https://godbolt.org/z/fKoYGdz88). If so, this might be expected behavior: The call to the `consteval` function would be an immediate invocation, which would itself be required to be a constant expression; since its definition is not reachable, the program would be ill-formed. But again, I couldn't say for sure without seeing an example - let me know if you have one, and I can take a look!

## Experimental flag?

@AaronBallman is correct that this change can break existing programs; indeed, it can even change the behavior of existing programs without breaking them, as demonstrated here: https://godbolt.org/z/cYnss3bMa .

That said, I find myself mildly against the idea of guarding this behind an experimental flag, for the reason that I don't think there will ever come a point where we feel comfortable flipping it to on-by-default. Therefore, I would prefer, if anything, to provide an opt-out flag for compatibility. But after chatting offline with @cor3ntin, I think it better to wait and see whether this change in fact causes grief for downstream users - we can always add a flag thereafter if we hear a lot of noise.

## Planned changes

I _believe_ that covers all of the concerns raised thus far (let me know if I've missed any!). With all of that said, here are the changes that I intend to further implement:
- Ensure that instantiation side-effects can occur in all cases where the language requires checking whether a variable's initializer is manifestly constant-evaluated.
- I don't think there's much value to the `Sema::getSemaProxy()` method; we'd might as well just construct the lightweight proxy object on the stack at each callsite. During offline discussions with @zygoloid, he agreed that this was a nice direction; I plan to update the PR accordingly.

And of course I'll continue to address any other feedback, and make any further changes requested from maintainers.

https://github.com/llvm/llvm-project/pull/205557