zygoloid wrote: > 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.
External callers of the `Eval*` functions in almost all cases should not be passing in this extra information -- existing callers of evaluation functions do *not* expect those functions to mutate the AST. Making the default behavior be a "pure" evaluation with a guarantee of no side effects seems like it's going to be the right default in almost all cases. We absolutely don't want existing users of Clang's API to suddenly start triggering AST mutation in calls that previously were performing pure evaluation of an expression for its value. > 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. I don't think so, because callers that don't have access to `Sema` should never be doing an evaluation that performs AST mutation. > 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. I'm not following the argument here. If the default is that you don't get AST mutation, then it seems to me that you would need to explicitly and visibly opt into getting AST mutation by passing the callbacks to 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
