davidstone wrote: I'll admit, I'm surprised this PR has any push-back at all since this feels like just applying a very generally accepted principle. Things should be const by default -- you need a reason to declare something as not const, you should never need a reason to declare something as const.
It's perfectly normal for a function named `setFoo` to accept a pointer to const: just because we are changing which Module something refers to does not mean that object needs to be able to modify the Module itself. It's just saying "Here is the thing we need to be able to read from later". Right now, a lot of the clang code is designed as "Here is a whole bunch of state that huge chunks of the program have access to", which means that understanding how any part works requires understanding how everything works -- you cannot reason about anything in isolation. This is a problem. The first step in fixing that is constraining who can modify the object. This commit is one of the sub-steps in that step. The next major commit I am working on tries to better encapsulate `Module` itself so that it gives out mutable access to less of its data. The biggest limitation to writing encapsulated classes is when they give out mutable references to any of their data because at that point you cannot control any invariants involving that piece of data. Once we have that, the understanding how the state of a Module changes over time requires understanding a significantly smaller part of the codebase, which will then allow another follow-up commit that changes `Module` to conceptually represent a module, rather than just being a chunk of state. At that point, understanding how a `Module` works will require just reading the source of `Module` rather than the source of `clang/`. https://github.com/llvm/llvm-project/pull/93493 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
