[cfe-commits] [patch] Honor -fno-pic, -fno-PIC, -fno-pie, -fno-PIE

[Chandler Carruth]

On Thu, Apr 5, 2012 at 12:49 AM, Nico Weber <thakis at chromium.org> wrote:

> Now with a driver-based test instead (addresses Joerg's comment too).
> Better?
>

Awesome. Nits:

--- test/Driver/fno-pic.c (revision 0)
+++ test/Driver/fno-pic.c (revision 0)
@@ -0,0 +1,5 @@
+// RUN: %clang -c %s -o %t.o -target i386-apple-darwin -### 2>&1 |
FileCheck %s --check-prefix=PIC_ON_BY_DEFAULT
+// PIC_ON_BY_DEFAULT: "-pic-level" "1"
+
+// RUN: %clang -c %s -target i386-apple-darwin -### -fno-pic 2>&1 |
FileCheck %s --check-prefix=FNO_PIC
+// FNO_PIC: "-mrelocation-model" "static"

why '-o %t.o' in the first, and not in the second? It shouldn't make a
difference with -###, was just curious about the inconsistency.

--- lib/Driver/Tools.cpp (revision 154058)
+++ lib/Driver/Tools.cpp (working copy)
@@ -1471,7 +1471,11 @@
                      Args.hasArg(options::OPT_fPIE) ||
                      Args.hasArg(options::OPT_fpie));
   bool PICDisabled = (Args.hasArg(options::OPT_mkernel) ||
-                      Args.hasArg(options::OPT_static));
+                      Args.hasArg(options::OPT_static)) ||
+                      Args.hasArg(options::OPT_fno_PIC) ||

I'm not great at reading indentation in patch files, so this may not be
quite right, but it looks like the indent is off for these to me...
specifically, the indent is the same for OPT_static and for OPT_fno_PIC,
when in fact, they have different groupings...

But actually, the different groupings are a no-op, as everything is ||
here. I think the parens only existed to make indentation nicer, so maybe
just remove the second karen after OPT_static to before the ';'?


Anyways, LGTM to submit with these style nits cleaned up. Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120405/1d285ac9/attachment.html>