[PATCH] D57546: [WebAssembly] Make segment/size/type directives optional in asm

[Wouter van Oortmerssen via Phabricator via llvm-commits]

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


================
Comment at: lib/MC/MCParser/WasmAsmParser.cpp:97
+    // auto WS = getContext().getWasmSection(Name, SectionKind::getText());
+    // getStreamer().SwitchSection(WS);
     return false;
----------------
dschuff wrote:
> dschuff wrote:
> > aardappel wrote:
> > > sbc100 wrote:
> > > > Why not just remove this?    Same below.
> > > I thought I'd leave this in at least for review, and potentially also to show "this is what this operation would do", since it is kinda odd that we parse the directive, then don't do anything with it.
> > > 
> > > I mean, I could remove all the parsing code and replace it with code that skips this directive, but I thought I'd leave it in, should we have a need for it later.
> > Is it really true that we never use the section directive? I guess for functions that makes sense but what about for data sections?
> WRT leaving in the commented-out code, the text comment could at least say something like "here is what *would* be here otherwise" so it's clear that the code is there intentionally.
We currently don't, as data sections aren't implemented yet. I'll revise this code if/when I implement those. And yes, I'll change the comment.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57546