[PATCH] D54875: [WebAssembly] Add support for the event section
Sam Clegg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 6 16:32:21 PST 2018
sbc100 added a comment.
This change is looking nice!
The only thing I'm not sure about is the header file named "InputEntity.h". This names doesn't mean much. Perhaps we can avoid the problem by just have `InputGlobal.h` and `InputEvent.h` as separate headers?
Aside form that an couple of nits this lgtm.
================
Comment at: include/lld/Common/LLVM.h:50
+ struct WasmEvent;
+ struct WasmEventType;
struct WasmRelocation;
----------------
nit: alphabetize
================
Comment at: test/wasm/Inputs/event-section1.ll:10
+}
+
----------------
nit: remove trailing newlines, or is that just phabricator?
================
Comment at: wasm/SymbolTable.cpp:153
+ const WasmSignature *OldSig = ExistingEvent->Signature;
+ if (NewType->Attribute != OldType->Attribute)
+ error("Event type mismatch: " + Existing->getName() + "\n>>> defined as " +
----------------
This reminds me that `Attribute` is a really bad name, but we don't need to block landing this CL on that name which is LLVM side.
Repository:
rLLD LLVM Linker
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54875/new/
https://reviews.llvm.org/D54875
More information about the llvm-commits
mailing list