[PATCH] D42520: Implement __attribute__((import_module("foo")))

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 28 10:22:23 PST 2018


sbc100 added a comment.

In general, this seems like a good idea.



================
Comment at: include/llvm/MC/MCSymbolWasm.h:39
       : MCSymbol(SymbolKindWasm, Name, isTemporary),
-        ModuleName("env") {}
+        ModuleName("__linker_resolve") {}
   static bool classof(const MCSymbol *S) { return S->isWasm(); }
----------------
To keep this change focused, perhaps this rename can be done separately.  

This would have several advantages:
1) Separate the addition of this new attribute from the bike-shedding around this default name
2) Allow with change to land without break lld (or requiring any lld change at all)
3) Possible avoid churns since we are looking to landing the symbol table change soon anyway which might eliminate this
4) Keep this change small

If we are going to bikeshed a new name here, I would go for something a little shorter, but I like the `__` prefix.  Perhaps just `__extern`?


Repository:
  rL LLVM

https://reviews.llvm.org/D42520





More information about the llvm-commits mailing list