[PATCH] D75306: [ms] [llvm-ml] Add initial MASM STRUCT/UNION support

Nico Weber via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 14 16:21:43 PDT 2020


thakis added a comment.

Thanks, the code looks close to done to me now. Can you add a tests that covers most of the error cases as well?



================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:3325
+  if (StructInProgress.empty()) {
+    // Initialize named data value
+    MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
----------------
.


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:3445
+
+// Initialize real data values
+bool MasmParser::emitRealValues(const fltSemantics &Semantics) {
----------------
.

…also in many other places. `curl https://reviews.llvm.org/file/data/mrzekpri33kb4kddo3tr/PHID-FILE-ejca6nujwdhbsxqgces2/D75306.diff | grep '^\+.*// [A-Z].*[^.]$'` shows some of them.


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:3698
+        continue;
+      } else {
+        FieldInitializers.emplace_back(Field.Contents.FT);
----------------
llvm style says no else after return/continue/break (https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code) (just put the else's body's contents right after the if and dedent)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75306





More information about the llvm-commits mailing list