[PATCH] D64880: ELF: Allow forward references to linked sections.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 18 00:06:05 PDT 2019


grimar accepted this revision.
grimar added a comment.

LGTM. A few "up to you" suggestions are below



================
Comment at: lld/ELF/InputFiles.cpp:657
+
+  for (size_t i = 0, e = objSections.size(); i < e; i++) {
+    if (this->sections[i] == &InputSection::discarded)
----------------
nit: we usually use `++i` form I think.


================
Comment at: lld/test/ELF/linkorder-forward-ref.test:6
+
+# Test that we accept forward sh_link references.
+
----------------
nit: for LLVM tools it became common to start comments from `##`.
Many tests in LLD also do that (it is actually where this style initially start spreading).
I'd suggest using it.


================
Comment at: lld/test/ELF/linkorder-forward-ref.test:20
+    Type:          SHT_PROGBITS
+    Flags:         [ SHF_ALLOC, SHF_EXECINSTR, SHF_LINK_ORDER ]
+    Link:          2
----------------
nit: seems you do not need `SHF_ALLOC`, `SHF_EXECINSTR` flags here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64880





More information about the llvm-commits mailing list