[PATCH] D23563: [RISCV 6/10] Add basic RISCVAsmParser

[Philip Reames via llvm-commits]

reames added a subscriber: reames.

================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:175
@@ +174,3 @@
+
+  static std::unique_ptr<RISCVOperand> CreateToken(StringRef Str, SMLoc S) {
+    auto Op = make_unique<RISCVOperand>(Token);
----------------
style: shouldn't this be createToken?

================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:203
@@ +202,3 @@
+  void addExpr(MCInst &Inst, const MCExpr *Expr) const {
+    if (!Expr)
+      Inst.addOperand(MCOperand::createImm(0));
----------------
This seems odd.  Is this a standard convention?  Or can we lift this check into the caller and assert the Expr is not null?

================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:346
@@ +345,3 @@
+
+bool RISCVAsmParser::ParseInstruction(ParseInstructionInfo &Info,
+                                      StringRef Name, SMLoc NameLoc,
----------------
This is a reasonable amount of parsing code without any associated tests.  Can we write even trivial tests for this code at this point?


https://reviews.llvm.org/D23563