[PATCH] D54096: [WebAssembly] Add support for the event section

[Sam Clegg via Phabricator via llvm-commits]

sbc100 accepted this revision.
sbc100 added inline comments.


================
Comment at: include/llvm/MC/MCSymbolWasm.h:74
   }
+  void setEventType(wasm::WasmEventType ET) { EventType = ET; }
 };
----------------
aardappel wrote:
> aheejin wrote:
> > @aardappel I changed the types of `GlobalType` and `EventType` to `llvm::Optional` and now we can remove those `GlobalTypeSet` and `EventTypeSet`. Thanks for the suggestion.
> awesome.. that's a solution I hadn't thought of.
lgtm, as long as making these Optional doesn't require the destructor being called.   IIRC we use of have a non-POD type in MCWasmSymbol before and we failed with msan because of that.  Do Optional POD types like this allocate?  I assume not.


================
Comment at: tools/yaml2obj/yaml2wasm.cpp:482
-      if (Type < LastType) {
-        errs() << "Out of order section type: " << Type << "\n";
-        return 1;
----------------
I feel like the section ordering constraint should still be enforces somewhere,  presumably when parsing the yaml into the in-memory sections.  But we also need to enforce it when parsing the binaries in WasmObjectFile and I'm OK with that being done in a later change.


Repository:
  rL LLVM

https://reviews.llvm.org/D54096