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

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: include/llvm/MC/MCSymbolWasm.h:27
+  wasm::WasmEventType EventType;
+  bool EventTypeSet = false;
 
----------------
aardappel wrote:
> aheejin wrote:
> > aardappel wrote:
> > > nicer to have an "invalid" member of `WasmEventType` ?
> > How? It is a struct in `include/llvm/BinaryFormat/Wasm.h`:
> > ```
> > struct WasmEventType {
> >   uint32_t Attribute;     
> >   uint32_t SigIndex;                                  
> > };
> > ```
> > And there are many other similar struct in that file.
> Ah sorry, I thought it was an enum. Still, having a bool just to indicate if another variable is set is not great, I guess there is no "default" value for `WasmEventType` ?
Yeah it's not great, but I'm not sure how we can work around it. `GlobalType` and `GlobalTypeSet` are the same case. If we make it a pointer then we can tell it's not set if it's a null pointer, but then it also creates another ownership problem because `MCSymbol`s are not deleted by `MCContext`, which was described in D52580.


Repository:
  rL LLVM

https://reviews.llvm.org/D54096