[cfe-commits] [patch] Honor -fno-pic, -fno-PIC, -fno-pie, -fno-PIE
thakis at chromium.org
Wed Apr 4 16:00:39 PDT 2012
On Wed, Apr 4, 2012 at 3:56 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 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).
> 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) ||
> 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!
All done, landed in r154064. Thanks!
More information about the cfe-commits