[PATCH] D15700: Split Undefined and UndefinedElf

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 22 01:04:36 PST 2015


grimar accepted this revision.
grimar added a comment.

LGTM with a few nits.


================
Comment at: ELF/OutputSections.cpp:1333
@@ -1331,1 +1332,3 @@
 template <class ELFT>
+static const typename llvm::object::ELFFile<ELFT>::Elf_Sym *
+getElfSym(SymbolBody &Body) {
----------------
I am not sure if this place is clang formatted (looks like not).

================
Comment at: ELF/Symbols.cpp:82
@@ +81,3 @@
+                     bool CanKeepUndefined)
+    : Undefined(SymbolBody::UndefinedKind, N, IsWeak, Visibility, false) {
+  this->CanKeepUndefined = CanKeepUndefined;
----------------
ruiu wrote:
> Pass CanKeepUndefined instead of false.
This false is about IsTls I think.
Thats the problem of these constructors - them are very similiar, but isTls/CanKeepUndefined differs what confuses. May be it is worth to keep both flags here ? One more excess argument is not good either though.


http://reviews.llvm.org/D15700





More information about the llvm-commits mailing list