[clang] Enable AST mutation in the constant evaluator (PR #115168)

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 25 11:33:31 PST 2024


zygoloid wrote:

If we do end up needing this (and it's looking increasingly likely that we will), I think the general approach of having a set of callbacks that gets passed into the constant evaluator is the right approach.

I think the older approach in this PR (a callback object that is explicitly passed to each AST operation that needs it) is significantly preferable to the newer approach (caching the callback in the `ASTContext`). The reason is that I think it's very important that we have a bright line visible at every call to the evaluator distinguishing

- incidental evaluations that we perform during various checking / diagnostic / code generation stages and that absolutely must not trigger AST mutations or any kind of visible behavior and
- the special "the language rules say this is manifestly constant evaluated" cases that should be able to perform AST mutations, that we need to be extremely careful to invoke at exactly the right times and in exactly the right cases and to invoke only once

If we're going to allow AST mutation from constant evaluation, we need very strong guard rails around it to make sure it doesn't happen accidentally. We're already passing in information to the evaluator for this distinction (to determine the value of `__builtin_is_constant_evaluated`), so switching to passing a pointer instead of an enumerator seems reasonable, and constructively guarantees we don't get this wrong inside the implementation of the evaluator.

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


More information about the cfe-commits mailing list