[PATCH] D152085: [llvm][MC] Add .pushsection/.popsection support to COFFAsmParser

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/MC/MCParser/COFFAsmParser.cpp:371
 // Subsections are not supported.
-bool COFFAsmParser::ParseDirectiveSection(StringRef, SMLoc) {
+bool COFFAsmParser::ParseSectionArguments(StringRef, SMLoc) {
   StringRef SectionName;
----------------
MaskRay wrote:
> While changing signature, consider fixing the function case to `camelCase`.
`parseSectionArguments`


================
Comment at: llvm/lib/MC/MCParser/COFFAsmParser.cpp:377
 
   unsigned Flags = COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
+                   COFF::IMAGE_SCN_MEM_READ | COFF::IMAGE_SCN_MEM_WRITE;
----------------
Don't format this unrelated part.


================
Comment at: llvm/lib/MC/MCParser/COFFAsmParser.cpp:442
+  if (!getStreamer().popSection())
+    return TokError(".popsection without corresponding .pushsection");
+  return false;
----------------
This error is not tested. See `llvm-mc ... --defsym ERR=1` in llcm/test/MC for error testing.


================
Comment at: llvm/test/MC/COFF/section.s:238
+
+# in section .data3
+.quad 4
----------------
`//`

don't mix different comment markers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152085