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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to