joerg added a subscriber: joerg.
joerg added a comment.
I agree in principle. The "-static" thing is already in the initial code, but
doesn't make sense to me.
================
Comment at: lib/Driver/Tools.cpp:2928
@@ +2927,3 @@
+/// Parses the various -fpic/-fPIC/-fpie/-fPIE arguments. Then,
+/// smooshes them together with platform defaults, to decide with
+/// whether this compile should be using PIC mode or not. Returns the
----------------
Drop the last with?
================
Comment at: lib/Driver/Tools.cpp:2930
@@ +2929,3 @@
+/// whether this compile should be using PIC mode or not. Returns the
+/// answer in a tuple of (RelocationModel, PICLevel, IsPIE).
+static std::tuple<llvm::Reloc::Model, int, bool>
----------------
Not a native speaker, but "returns as tuple" sounds more natural?
Why is the PICLevel signed?
================
Comment at: lib/Driver/Tools.cpp:2998
@@ +2997,3 @@
+ // both PIC and PIE are disabled. Any PIE option implicitly enables PIC
+ // at the same level.
+ Arg *LastPICArg = Args.getLastArg(options::OPT_fPIC, options::OPT_fno_PIC,
----------------
No need to rant about GCC behavior that much. Just state that older GCC
versions provided different (inconsistent behavior).
================
Comment at: lib/Driver/Tools.cpp:3022
@@ +3021,3 @@
+ // Introduce a Darwin-specific hack. If the default is PIC but the flags
+ // specified while enabling PIC enabled level 1 PIC, just force it back to
+ // level 2 PIC instead. This matches the behavior of Darwin GCC (based on my
----------------
Difficult to parse. Please rewrite the second sentence.
================
Comment at: lib/Driver/Tools.cpp:3032
@@ +3031,3 @@
+ PIC = PIE = false;
+ if (Args.hasArg(options::OPT_static))
+ PIC = PIE = false;
----------------
This check doesn't make sense to me. When using just -S, -static is completely
ignored. So why should it be relevant here?
================
Comment at: lib/Driver/Tools.cpp:3087
@@ -2943,1 +3086,3 @@
const ArgList &Args, const char *LinkingOutput) const
{
+ std::string TripleStr = getToolChain().ComputeEffectiveClangTriple(Args);
+ const llvm::Triple Triple(TripleStr);
----------------
This part looks like a clean up that can be applied separately first?
http://reviews.llvm.org/D11845
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
