More instructions get named #4615
Conversation
|
I'm happy to hear suggestions for changing the strings, particularly if you have a good idea for making names shorter. For example, maybe |
toolchain/sem_ir/inst_namer.cpp
Outdated
| case CARBON_KIND(InterfaceWitnessAccess inst): { | ||
| std::string name; | ||
| llvm::raw_string_ostream out(name); | ||
| out << "elt" << inst.index.index; |
There was a problem hiding this comment.
We use "elt" in a couple of other places. Is there a more specific name we could give this?
There was a problem hiding this comment.
We could add a letter to the beginning, like ielt, I didn't do that at first since I was worried that would be too mysterious. I've made that change, so you can see how it looks.
There was a problem hiding this comment.
I agree it's a little mysterious from the value name alone, but I think from the instruction name interface_witness_access it's reasonably clear what it means. Maybe we'll find a better name later but this seems good enough for now.
There was a problem hiding this comment.
We do tuple.elem. Maybe elem since it's consistent with current use and the C++ style guide advises against abbreviation by removing letters in the middle?
Maybe impl.elem? (similar to how tuple access gives a tuple element, witness access giving an impl element)
toolchain/sem_ir/inst_namer.cpp
Outdated
| case CARBON_KIND(TupleAccess inst): { | ||
| std::string name; | ||
| llvm::raw_string_ostream out(name); | ||
| out << "telt" << inst.index.index; |
There was a problem hiding this comment.
Note, I'd suggest a similar rename here
Goal is to reduce churn in names in test updates (by churning a lot of them in this PR).