Endilll wrote: @zygoloid I definitely don't disagree with your points. After extensive discussions with other maintainers (Shafik, Corentin, Erich, Aaron), I went from passing the callback explicitly to very implicit approach, because of reasons that lie outside of usage of AST in Clang itself, and revolve around tools like clang-tidy: 1. Asking users to pass additional parameter to every `Eval*()` function makes a bad transition story for users that wish to upgrade to a version of Clang that has changed the interface in this way. 2. The transition story becomes even worse when we consider the fact that `Sema` is the only reasonable implementation of this callback, whereas clang-tidy doesn't typically have access to `Sema`. So we're asking users to pass an implementation of interface they have barely any chance to implement. 3. The transition story could be improved by specifying a default for callback parameter. It eases the transition, but completely murders the intent that enabling AST mutation during constant evaluation has to be visible.
So the approach we took is like (3) above, but maxing out on hiding this new callback from the API. If constant evaluation is triggered without `Sema` available, nothing changes. If `Sema` is created, then you get full C++26 constant evaluation capabilities, including AST mutation. This also means that future additions to constant evaluator that need AST mutation cannot blindly assume that AST mutation is available. We also identified that issues like #73232 can leverage this new callback. To my understanding, this was one of the reasons why idea to limit calls to callback to C++26 and newer language modes didn't have traction among maintainers I've been ~~nagging~~ discussing this PR with (Shafik, Corentin, Erich, Aaron). I think there's also understanding among maintainers that this PR is strictly worse that status quo whichever approach is taken. No one, including me, enjoys storing a callback in effectively a global variable (`ASTContext` is available almost everywhere). As long as P2996 comes with sides effects in constant evaluation, we're just picking our poison. CC @AaronBallman in case I forgot something or misrepresented your opinion on situation about external users of Clang. 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
