It's an internal API, there is no need to have const functions.
We often unintentionally work around constness anyway by using T& data members which can be modified in const functions.
Details
- Reviewers
arsenm foad aemerson zuban32 mpaszkowski
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
60,030 ms | x64 debian > MLIR.Examples/standalone::test.toy |
Event Timeline
Additional context: While refactoring this area for some upcoming diffs (removing the deprecated GICombiner backend), I changed some data members from T& to T and got a lot of errors due to this.
While I usually favor const-correctness as much as possible, it's really never respected here. We always use references or pointers, which means we can always call non-const functions on those even in a const function.
Example:
struct Foo { int foo() { return 0; } }; struct Bar { Foo &F; void bar() const { F.foo(); } };
So there's two solutions, the first one is for me to use mutable whenever I run into a case like this when refactoring, or we can just eliminate const where it doesn't make sense.
I'm fine with both, but I think it's cleaner to just eliminate these "fake const" that don't really guarantee anything in terms of semantics. Hence why this diff is a RFC.
e.g. we constantly mutate state - directly or indirectly - in those functions through the MIR builder, MRI, Observers, etc.
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.h | ||
---|---|---|
102 | I'm not seeing the context how this helps. I think things should be kept const, especially since all the others here are |
It's a trivial example, in most cases we'd be calling CombinerHelper functions or GlobalISelObserver functions.
I'll double check if I can make the offending function consts, and keep mutable around for now. I'll come back to this if I still think it makes sense afterwards
I'm not seeing the context how this helps. I think things should be kept const, especially since all the others here are