AaronBallman wrote: > @zygoloid I think we ended up concluding that > > * The tooling situation will not change (When Sema is not available > mutation can't happen and we provide a default no-op implementation of the > mutation function) > > * It's easy to check in `SemaProxyImpl` (or wherever it's called from ) > that define class etc only happen in what P2996 calls a "plainly evaluated > constant evaluated context" (constexpr initialization + consteval block as of > the Poland meeting) > > > I initially was of the same opinion as you, but I think we would have to > modify most call sites, and it's unclear that it would buy us anything (On > the other hand, I'm not thrilled about a pointer to Sema being stashed in > `ASTContext`, just because it feels weird, but it's not an issue in practice)
I'm of the same opinion. My logic when I proposed this design to Vlad was: 1) We need a callback somewhere; it can be explicitly part of the interfaces or it can be hidden from view. 2) Normally, explicit interfaces are better; however, this interface can only realistically be implemented by one thing: `Sema`. No user of (e.g.) clang-tidy is going to implement their own semantic layering just so constant evaluation of side effecting reflection works. 3) We want to cause as little disruption as possible for downstream consumers of the AST library. That means: no requirement to link clangSema in to clangAST (layering violations) and it means that interface changes should come with enough benefit to warrant breaking every downstream interface trying to evaluate constant expressions (of which there are plenty). 4) Given that `Sema` is the only viable way to implement the callback and that we don't want to change the interfaces without benefit, it didn't make sense to explicitly modify all the constant expression evaluating APIs to thread that through them. Instead, it seemed less invasive to have a hidden pointer to the callback somewhere and if that pointer is null, we react as though the side effect simply never took place (or we could diagnose more explicitly if we wanted). This approach should have the least impact on downstream consumers because the AST should be fully formed before tools like clang-tidy, etc are evaluating the AST (so side effects should have already taken place), so the evaluation results should Just Work⢠in most cases, so long as the execution path was exercised by the compiler. It's not an ideal design, but I think it strikes the right balance. I wonder if there's a way we can have more of a bright line between the APIs which can mutate state and ones which can't. For example, could we come up with an attribute we require to be written on the API and is checked via a clang-tidy check? > PS: Note that it seems to still be unclear how we should handle try > evaluation (destructors, vlas), when the evaluation fails after side effects > are produced +1, though my understanding was that if evaluation fails, then the program should be ill-formed, and so what happens with destructors, etc is not really important. Did I understand wrong though? 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
