[PATCH] D119597: [clang-format][NFC] Give State.Stack.back() a meaningful name

Sat Feb 12 02:28:58 PST 2022

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