Code view type record "integer" type values are currently emitted in decimal format. This change will display these values in hex format for better readability.
Diff Detail
- Repository
- rL LLVM
Event Timeline
llvm/include/llvm/MC/MCExpr.h | ||
---|---|---|
137 | I'm not sure we should add this in here. I added @pcc for a second opinion. Somehow I feel like the MCExpr class hierarchy shouldn't be concerned with syntactic details, and the printer should receive some kind of printing policy instead. However, I see this field in MCSymbolRefExpr: /// Specifies how the variant kind should be printed. const unsigned UseParensForSymbolVariant : 1; So, clearly printing options are already held in MCExpr objects. I could go either way, so I figured I'd ask Peter. | |
159 | Functions should start with lower case letters: | |
llvm/include/llvm/MC/MCStreamer.h | ||
631 | I suggest running git clang-format to address this and other formatting issues. | |
llvm/lib/MC/MCExpr.cpp | ||
49 | There should be a space between the if and (, clang-format will fix it |
llvm/include/llvm/MC/MCExpr.h | ||
---|---|---|
137 | I guess MCExprs are sort of like clang expressions in that they are in fact syntactic, e.g. you can express the number 2 as an MCBinaryExpr 1+1 if you wanted. We could even preserve the number format given assembly input, although it's not clear how useful that would be. So I wouldn't be too opposed to storing the format here. |
lgtm with the explicit marker removed. Thanks!
llvm/include/llvm/MC/MCExpr.h | ||
---|---|---|
137 | Alright, sounds good to me. It gives us the flexibility to print numbers contained in expressions in hex as well, so that's nice. | |
142 | I believe explicit on a multi-argument constructor does nothing, so I would remove it. It's only to prevent excessive unintended automatic conversions. |
I'm not sure we should add this in here. I added @pcc for a second opinion. Somehow I feel like the MCExpr class hierarchy shouldn't be concerned with syntactic details, and the printer should receive some kind of printing policy instead. However, I see this field in MCSymbolRefExpr:
So, clearly printing options are already held in MCExpr objects. I could go either way, so I figured I'd ask Peter.