On Wed, Apr 4, 2012 at 3:56 PM, Chandler Carruth <[email protected]> wrote: > On Thu, Apr 5, 2012 at 12:49 AM, Nico Weber <[email protected]> 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!
All done, landed in r154064. Thanks! _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
