[PATCH] D61811: [WebAssembly] Refactor synthetic sections and relocation processing. NFC.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/wasm/OutputSections.h:51
   std::string Name;
+  OutputSectionSymbol* SectionSym = nullptr;
 
----------------
nit: `OutputSectionSymbol* ` -> `OutputSectionSymbol *`


================
Comment at: lld/wasm/Relocations.cpp:38
+    // Other relocation types all have a corresponding symbol
+    auto *Sym = File->getSymbols()[Reloc.Index];
+
----------------
`auto` -> `Symbol`


================
Comment at: lld/wasm/Relocations.cpp:58-59
+    case R_WASM_MEMORY_ADDR_REL_SLEB:
+      if (!Config->Relocatable) {
+        if (Sym->isUndefined() && !Sym->isWeak()) {
+          error(toString(File) + ": cannot resolve relocation of type " +
----------------
nit: you can merge the two `if`s into one `if`.


================
Comment at: lld/wasm/Relocations.cpp:75
+        // they would require absolute symbol addresses at link time.
+        Symbol *Sym = File->getSymbols()[Reloc.Index];
+        error(toString(File) + ": relocation " +
----------------
Isn't this the same as line 38?


================
Comment at: lld/wasm/Relocations.cpp:86
+        // requires the symbols to have GOT entires.
+        auto* Sym = File->getSymbols()[Reloc.Index];
+        if (requiresGOTAccess(Sym))
----------------
Ditto


================
Comment at: lld/wasm/Symbols.h:20-21
 
+extern const char *DefaultModule;
+extern const char *FunctionTableName;
+
----------------
It would be nice to explain them as comment.


================
Comment at: lld/wasm/Symbols.h:45
     SectionKind,
+    OutputSectionKind,
     UndefinedFunctionKind,
----------------
It seems like a domain error that we have "output section kind" of a symbol, because symbols are not sections and vice versa. What is this?


================
Comment at: lld/wasm/Symbols.h:201
 
-class SectionSymbol : public Symbol {
+class OutputSectionSymbol : public Symbol {
 public:
----------------
Ah, okay, so this class represents a section symbol... but it is not clear why we have two different section symbol types. Could you add comments?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61811/new/

https://reviews.llvm.org/D61811