[PATCH] D77919: [MC][ELF] Add section flags to diagnostic

Fri Apr 10 17:47:47 PDT 2020

bcain marked 2 inline comments as done.
bcain added inline comments.

================
Comment at: llvm/lib/MC/MCParser/ELFAsmParser.cpp:646
+    const std::string ExpectedFlagsText =
+        Twine(StringRef((ExpectedFlags & ELF::SHF_ALLOC) ? "a" : "") +
+              StringRef((ExpectedFlags & ELF::SHF_WRITE) ? "w" : "") +
----------------
MaskRay wrote:
> There are actually a lot more section flags, e.g. SHF_MERGE ('M') SHF_LINK_ORDER ('o') SHF_EXCLUDE ('e'). This patch will leave an empty string for an unknown flag.
Ok, I'll fix it by making it exhaustive.

================
Comment at: llvm/lib/MC/MCParser/ELFAsmParser.cpp:650
+            .str();
+    Error(loc, "changed section flags for `" + SectionName + "', expected: \"" +
+                   ExpectedFlagsText + "\"");
----------------
MaskRay wrote:
> A backquote (binutils uses it sometimes) is actually uncommon in LLVM diagnostics. Both unquoted and `'`-quoted section names are common.
IMO quoting it one way or another makes more sense than not.  I can use single-quotes if that's more conventional.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77919