[WebAssembly] Assembler: Improve section parsing.

[sbc100]

lib/MC/MCParser/WasmAsmParser.cpp

163 ↗

(On Diff #207115)

Can you explain why this code is not needed and the commend isn't needed anymore?

[aardappel]

lib/MC/MCParser/WasmAsmParser.cpp

163 ↗

(On Diff #207115)

The section type parsing was actually duplicated code: the exact same thing is done by the StringSwitch above.

The "just ignore this section" is turned into an error, that was originally not an error just to get things going, but now I want to be more exact about our error checking and what we support.

llvm/trunk/lib/MC/MCParser/WasmAsmParser.cpp

if (Parser->parseIdentifier(Name))
       return TokError("expected identifier in directive");
 
     if (expect(AsmToken::Comma, ","))
       return true;
 
     if (Lexer->isNot(AsmToken::String))
       return error("expected string in directive, instead got: ", Lexer->getTok());
 
-    SectionKind Kind = StringSwitch<SectionKind>(Name)
-                       .StartsWith(".data", SectionKind::getData())
-                       .StartsWith(".rodata", SectionKind::getReadOnly())
-                       .StartsWith(".text", SectionKind::getText())
-                       .StartsWith(".custom_section", SectionKind::getMetadata());
+    auto Kind = StringSwitch<Optional<SectionKind>>(Name)
+                    .StartsWith(".custom_section", SectionKind::getMetadata())
+                    .StartsWith(".bss", SectionKind::getBSS())
+                    .Default(Optional<SectionKind>());
+    if (!Kind.hasValue())
+      return Parser->Error(Lexer->getLoc(), "unknown section kind: " + Name);
 
-    MCSectionWasm* Section = getContext().getWasmSection(Name, Kind);
+    MCSectionWasm *Section = getContext().getWasmSection(Name, Kind.getValue());
 
     // Update section flags if present in this .section directive
     bool Passive = false;
     if (parseSectionFlags(getTok().getStringContents(), Passive))
       return true;
 
     if (Passive) {
       if (!Section->isWasmData())
         return Parser->Error(getTok().getLoc(),
                              "Only data sections can be passive");
       Section->setPassive();
     }
 
     Lex();
 
     if (expect(AsmToken::Comma, ",") || expect(AsmToken::At, "@") ||
         expect(AsmToken::EndOfStatement, "eol"))
       return true;
-    struct SectionType {
-      const char *Name;
-      SectionKind Kind;
-    };
-    static SectionType SectionTypes[] = {
-        {".text", SectionKind::getText()},
-        {".rodata", SectionKind::getReadOnly()},
-        {".data", SectionKind::getData()},
-        {".custom_section", SectionKind::getMetadata()},
-        // TODO: add more types.
-    };
-    for (size_t I = 0; I < sizeof(SectionTypes) / sizeof(SectionType); I++) {
-      if (Name.startswith(SectionTypes[I].Name)) {
-        auto WS = getContext().getWasmSection(Name, SectionTypes[I].Kind);
-        getStreamer().SwitchSection(WS);
-        return false;
-      }
-    }
-    // Not found, just ignore this section.
-    // For code in a text section WebAssemblyAsmParser automatically adds
-    // one section per function, so they're optional to be specified with
-    // this directive.
-    return false;
-  }
+
+    auto WS = getContext().getWasmSection(Name, Kind.getValue());
 
   // TODO: This function is almost the same as ELFAsmParser::ParseDirectiveSize
   // so maybe could be shared somehow.
   bool parseDirectiveSize(StringRef, SMLoc) {
     StringRef Name;
     if (Parser->parseIdentifier(Name))
       return TokError("expected identifier in directive");
     auto Sym = getContext().getOrCreateSymbol(Name);

llvm/trunk/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

#include "WebAssemblyGenAsmMatcher.inc"
 
   void doBeforeLabelEmit(MCSymbol *Symbol) override {
     // Start a new section for the next function automatically, since our
     // object writer expects each function to have its own section. This way
     // The user can't forget this "convention".
     auto SymName = Symbol->getName();
     if (SymName.startswith(".L"))
       return; // Local Symbol.
+    // Only create a new text section if we're already in one.
+    auto CWS = cast<MCSectionWasm>(getStreamer().getCurrentSection().first);
+    if (!CWS || !CWS->getKind().isText())
+      return;
     auto SecName = ".text." + SymName;
     auto WS = getContext().getWasmSection(SecName, SectionKind::getText());
     getStreamer().SwitchSection(WS);
   }
 
   void onEndOfFunction() {
     // Automatically output a .size directive, so it becomes optional for the
     // user.