[PATCH] D64679: Display codeview type record values in hex representation instead of decimal

[NILANJANA BASU via Phabricator via llvm-commits]

nilanjana_basu marked 5 inline comments as done.
nilanjana_basu added inline comments.


================
Comment at: llvm/include/llvm/MC/MCExpr.h:137
   int64_t Value;
+  bool PrintInHex = false;
 
----------------
pcc wrote:
> rnk wrote:
> > 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.
> 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.
I did it this way because the print function prints different types of MCExpr, which might have different syntactic needs. Changing the function parameter only for printing MCConstantExpr seemed awkward.


================
Comment at: llvm/include/llvm/MC/MCExpr.h:157
 
+  bool UseHexFormat() const { return PrintInHex; }
+
----------------
rnk wrote:
> Functions should start with lower case letters:
> https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
Fixed


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64679/new/

https://reviews.llvm.org/D64679