[PATCH] D37412: [X86][MS-InlineAsm] Extended support for variables / identifiers on memory / immediate expressions

[coby via Phabricator via llvm-commits]

coby added inline comments.


================
Comment at: include/llvm/MC/MCParser/MCAsmParser.h:43
+    VarKind       // Variable.
+  };
+  // Represents an Enum value
----------------
RKSimon wrote:
> Preferred enum style is
> ```
>  enum IdKind {
>     IK_Invalid,      // Initial state. Unexpected after a successful parsing.
>     IK_Label,    // Function/Label reference.
>     IK_EnumVal,  // Value of enumration type.
>     IK_Var       // Variable.
>   };
> ```
accepted


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1275
     unsigned Scale, SMLoc Start, SMLoc End, unsigned Size, StringRef Identifier,
-    InlineAsmIdentifierInfo &Info) {
+    const InlineAsmIdentifierInfo &Info) {
   // If we found a decl other than a VarDecl, then assume it is a FuncDecl or
----------------
RKSimon wrote:
> This looks like its a NFC commit that can be done separately 
can you be a bit more specific regarding the exact part which can be commited as a NFC?
The only one i'm trivially spotting is the encapsulation of the constant symbolic reference inside of 'onIdentifierExpr', when parsing assembly (non inline-asm) content, rather than prior calling it - was that the issue you've meant? 


Repository:
  rL LLVM

https://reviews.llvm.org/D37412