[PATCH] D53842: [WebAssembly] Parsing missing directives to produce valid .o

[Kristina Brooks via Phabricator via llvm-commits]

kristina added inline comments.


================
Comment at: include/llvm/ADT/StringSwitch.h:214
+    return std::move(Result);
+  }
+
----------------
aardappel wrote:
> dschuff wrote:
> > aardappel wrote:
> > > Since this is a core LLVM class, should I get someone in particular to review? Or am I better off writing the calling code differently?
> > For our use case, we have `T = wasm::WasmSymbolType` and `R` defaults to `T`. Could we set `R` to `Optional<wasm::WasmSymbolType>` and just use `Default` instead of `NoDefault`?
> That doesn't work because the existing `Default` always returns a `T`, i.e. there is no way to indicate a null `Optional`.
The general convention is to have an "invalid" type as a default which can be explicitly checked for, something like `wasm::WASM_SYMBOL_TYPE_INVALID`. IMO if the enum has no default value and yet it's possible for it to end up in an "invalid" state, it *should* have a default value, which makes it much easier to understand the code.


Repository:
  rL LLVM

https://reviews.llvm.org/D53842