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

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 26 13:50:30 PST 2024


zygoloid wrote:

> Hey Richard - We added idempotency to `define_aggregate` in response to concern from Clang maintainers that it could interact poorly with `clang-repl`. The argument was that idempotency would provide a better experience for a user that wanted to re-evaluate an expression that was previously submitted for evaluation.

I see. I've given it some thought, and I don't agree that this argument should govern the behavior of `define_aggregate`. I would expect
```c++
struct S;
struct S { ... };
```
to behave exactly the same in a C++ interpreter / REPL as
```c++
struct S;
consteval { define_aggregate(^^S, {}); }
```
I think not following that principle would create unnecessary work for the implementation and confusion for users, as well as harming the general idea that code generated by metaprogramming works the same way as code generated by source declarations.

If, for a syntactic struct definition, an implementation can "undo" defining the struct and rewind back to before the definition, then I'd expect the same behavior for a metaprogramming struct definition (and your proposal shouldn't try to get in the way of that or dictate the behavior in that case, because it's outside the scope of the C++ standard). Conversely, if the implementation can't undo it in one case, I'd expect it to undo it in neither case. More broadly, if this is a real problem for an interpreter, then it's not unique to `define_aggregate` and applies to all syntactic and metaprogramming-introduced declarations and definitions, and such an interpreter should endeavor to use the same solution in all the places where the problem appears -- but, again, that situation and its chosen solution are outside the scope of the C++ standard and we don't need rules in the standard to enable the implementation to solve this problem.

Instead of supporting some kind of an "undo" mechanism, it might make sense for an interpreter to treat definitions as idempotent in general (perhaps with a special rule that we use the most recent body for a *function* definition). For such an implementation that makes that choice, it would seem appropriate for `define_aggregate` to also be idempotent. But that should be a specific rule for that implementation, not a rule that appears in the standard. And we certainly shouldn't change the standard to require this unusual implementation-specific rule to be implemented by all C++ implementations, because it might better fit a non-conforming model that `clang-repl` *might* adopt (and as far as I'm aware, we haven't chosen to make definitions idempotent in `clang-repl`).

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


More information about the cfe-commits mailing list