[PATCH] D92507: [ms] [llvm-ml] Add support for ALIGN, EVEN, and ORG directives

[Eric Astor via Phabricator via llvm-commits]

epastor added inline comments.


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:4639
+  // Always emit an alignment here even if we throw an error.
+  bool ReturnVal = false;
+
----------------
thakis wrote:
> Do you expect `parseDirectiveEven()` to become longer in the future? Why doesn't this do
> 
> ```
>   if (emitAlignTo(2))
>     return addErrorSuffix(" in even directive");
> ```
Simplified. Thanks!


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:4392
   StructInfo &ParentStruct = StructInProgress.back();
   if (Structure.Name.empty()) {
     const size_t OldFields = ParentStruct.Fields.size();
----------------
thakis wrote:
> can you add a comment here saying what an empty field name means? ("fields in nameless struct get added to containing struct")
Added for clarity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92507