-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document StepStack #4687
Document StepStack #4687
Conversation
toolchain/sem_ir/stringify_type.cpp
Outdated
enum Kind : uint8_t { | ||
Inst, | ||
FixedString, | ||
ArrayBound, | ||
Name, | ||
}; | ||
|
||
// An individual step in the stack. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all a bit abstract; I think it would help me to have more specifics about what a "step" represents. Maybe something like this?
// An individual step in the stack. | |
// An individual step in the stack, which prints some component of a type name. |
(A more specific name like "PrintStep" might also help, but that may be out of scope here.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW in the name direction, I think this is maybe okay right now because the file is just for stringification. But it might be worth it to rename StepStack
to be Stringifier
, refactoring StringifyTypeExpr
to use a member function so that all the push/pop logic would be private. (either way felt out-of-scope)
Co-authored-by: Geoff Romer <gromer@google.com>
Also add underscores to member names, and make
PushSpecificId
private since it's not used outsidePushEntityName