[PATCH] [lld] Fix the ELF shared library build targets
Rui Ueyama
ruiu at google.com
Thu Jan 22 13:49:24 PST 2015
In this patch you moved lib/ReaderWriter/ELF/<arch>/<arch>LinkingContext to lib/ReaderWriter/ELF. Why did you have to do that? Because of that change, the size of this diff is large, so it is not easy to find the answer myself. Could you elaborate that a bit?
REPOSITORY
rL LLVM
================
Comment at: include/lld/ReaderWriter/ELF/Targets.h:21
@@ +20,2 @@
+
+#endif
----------------
I don't agree that this is a good idea. We should generally avoid transitive inclusions. If a source file depends on a bunch of headers, it actually depends on all of them. Hiding that dependency by replacing with one #include doesn't make things clean but makes things slightly worse.
================
Comment at: lib/Driver/GnuLdDriver.cpp:343
@@ +342,3 @@
+ default:
+ return nullptr;
+ }
----------------
Legitimate code should never reach here, no? If so, please use llvm_unreachable.
http://reviews.llvm.org/D7119
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list