[PATCH] D36793: [X86AsmParser] Refactoring, (almost) NFC.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:339-377
+    IntelExprStateMachine() :
+      State(IES_INIT), PrevState(IES_ERROR), BaseReg(0), IndexReg(0),
+      TmpReg(0), Scale(1), Imm(0), Sym(nullptr), BracCount(0), MemExpr(false) {
+      Info.clear();
+    }
+
+    void addImm(int64_t imm) {
----------------
Can you clang-format this?


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:604
+    bool onIdentifierExpr(const MCExpr *SymRef, StringRef SymRefName,
+      StringRef &ErrMsg) {
       PrevState = State;
----------------
formatting


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:606
       PrevState = State;
+      bool HasError = (bool)Sym;
       switch (State) {
----------------
I think `HasSymbol` would be a better name for this, and `Sym != nullptr` is a more readable way to force a boolean conversion.


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1708
 
+bool X86AsmParser::ParseIntelSizeDirective(unsigned &Size) {
+  Size = StringSwitch<unsigned>(getTok().getString())
----------------
I think `ParseIntelMemoryOperandSize` would be a better name for this.


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1790
+
+  // RegNo <> 0 specifies a valid segment register,
+  // and we are parsing a segment override
----------------
Use `!=` instead of `<>`, it's not commonly used.


Repository:
  rL LLVM

https://reviews.llvm.org/D36793