HazardyKnusperkeks added a comment. In D119597#3316643 <https://reviews.llvm.org/D119597#3316643>, @MyDeveloperDay wrote:
> I like this change for clarity reasons, my only concern and it's not based on > evidence is what if any of these functions get passed in State, and then they > themselves alter the State.Stack? > > In most cases I'd expect CurrentState to always be correct, but doesn't it > only represent the state of the stack as it was at the beginning of the > function > > Isn't the point of State.Stack.back() to get the current state? if you do > that with a variable like `CurrentState` or just have > `State.getCurrentState()` I don't know but the later seems a good step > because you could use that in even more places > > i.e. > > `unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent;` > > `unsigned NestedBlockIndent = State.getCurrentState().NestedBlockIndent;` Currently everywhere where I have created the reference there are no changes to the stack afterwards. If someone will change that in the future they will get massive test failures, that I can promise from experience in this field. My reason for this change was debugging, I had a really hard time to understand what all these values and their changes (and why they change) mean. With the local variable I could easily monitor the values in my debugger view. If we replace one function call `State.Stack.back()` with `State.getCurrentState()` we are back at square one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119597/new/ https://reviews.llvm.org/D119597 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
