[cfe-commits] r154776 - in /cfe/trunk/test: Analysis/objc-bool.m Headers/typedef_guards.c Lexer/utf-16.c Preprocessor/pragma_sysheader.c Sema/surpress-deprecated.c SemaCXX/warn-everthing.cpp

Tue Apr 17 09:49:08 PDT 2012

Le 16 avril 2012 20:09, David Blaikie <dblaikie at gmail.com> a écrit :

> On Mon, Apr 16, 2012 at 11:03 AM, Chandler Carruth <chandlerc at google.com>
> wrote:
> > On Mon, Apr 16, 2012 at 7:53 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>
> >> On Mon, Apr 16, 2012 at 10:43 AM, Chandler Carruth <
> chandlerc at google.com>
> >> wrote:
> >> > On Mon, Apr 16, 2012 at 7:38 PM, David Blaikie <dblaikie at gmail.com>
> >> > wrote:
> >> >>
> >> >> On Mon, Apr 16, 2012 at 10:29 AM, Matthieu Monrocq
> >> >> <matthieu.monrocq at gmail.com> wrote:
> >> >> >
> >> >> >
> >> >> > Le 16 avril 2012 00:09, David Blaikie <dblaikie at gmail.com> a
> écrit :
> >> >> >
> >> >> >> Author: dblaikie
> >> >> >> Date: Sun Apr 15 17:09:44 2012
> >> >> >> New Revision: 154776
> >> >> >>
> >> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=154776&view=rev
> >> >> >> Log:
> >> >> >> Fix tests that weren't actually verifying anything.
> >> >> >>
> >> >> >> Passing -verify to clang without -cc1 or -Xclang silently passes
> >> >> >> (with
> >> >> >> a
> >> >> >> printed warning, but lit doesn't care about that). This change
> adds
> >> >> >> -cc1
> >> >> >> or,
> >> >> >> as is necessary in one case, -Xclang to fix this so that these
> tests
> >> >> >> are
> >> >> >> actually verifying as intended.
> >> >> >>
> >> >> >> I'd like to change the driver so this kind of mistake could not be
> >> >> >> made,
> >> >> >> but
> >> >> >> I'm not entirely sure how. Further, since the driver only warns
> >> >> >> about
> >> >> >> unknown
> >> >> >> flags in general, we could have similar bugs with a misspellings
> of
> >> >> >> arguments
> >> >> >> that would be nice to find.
> >> >> >>
> >> >> >> Modified:
> >> >> >>    cfe/trunk/test/Analysis/objc-bool.m
> >> >> >>    cfe/trunk/test/Headers/typedef_guards.c
> >> >> >>    cfe/trunk/test/Lexer/utf-16.c
> >> >> >>    cfe/trunk/test/Preprocessor/pragma_sysheader.c
> >> >> >>    cfe/trunk/test/Sema/surpress-deprecated.c
> >> >> >>    cfe/trunk/test/SemaCXX/warn-everthing.cpp
> >> >> >>
> >> >> >> Modified: cfe/trunk/test/Analysis/objc-bool.m
> >> >> >> URL:
> >> >> >>
> >> >> >>
> >> >> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc-bool.m?rev=154776&r1=154775&r2=154776&view=diff
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >>
> ==============================================================================
> >> >> >> --- cfe/trunk/test/Analysis/objc-bool.m (original)
> >> >> >> +++ cfe/trunk/test/Analysis/objc-bool.m Sun Apr 15 17:09:44 2012
> >> >> >> @@ -1,4 +1,4 @@
> >> >> >> -// RUN: %clang --analyze %s -o %t -verify
> >> >> >> +// RUN: %clang --analyze %s -o %t -Xclang -verify
> >> >> >>
> >> >> >>  // Test handling of ObjC bool literals.
> >> >> >>
> >> >> >>
> >> >> >> Modified: cfe/trunk/test/Headers/typedef_guards.c
> >> >> >> URL:
> >> >> >>
> >> >> >>
> >> >> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/typedef_guards.c?rev=154776&r1=154775&r2=154776&view=diff
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >>
> ==============================================================================
> >> >> >> --- cfe/trunk/test/Headers/typedef_guards.c (original)
> >> >> >> +++ cfe/trunk/test/Headers/typedef_guards.c Sun Apr 15 17:09:44
> 2012
> >> >> >> @@ -1,4 +1,4 @@
> >> >> >> -// RUN: %clang -fsyntax-only -verify %s
> >> >> >> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> >> >> >>
> >> >> >>  // NULL is rdefined in stddef.h
> >> >> >>  #define NULL ((void*) 0)
> >> >> >>
> >> >> >> Modified: cfe/trunk/test/Lexer/utf-16.c
> >> >> >> URL:
> >> >> >>
> >> >> >>
> >> >> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/utf-16.c?rev=154776&r1=154775&r2=154776&view=diff
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >>
> ==============================================================================
> >> >> >> --- cfe/trunk/test/Lexer/utf-16.c (original)
> >> >> >> +++ cfe/trunk/test/Lexer/utf-16.c Sun Apr 15 17:09:44 2012
> >> >> >> @@ -1,4 +1,4 @@
> >> >> >> -// RUN: not %clang %s -fsyntax-only -verify
> >> >> >> +// RUN: %clang_cc1 %s -fsyntax-only -verify
> >> >> >>  // rdar://7876588
> >> >> >>
> >> >> >>  // This test verifies that clang gives a decent error for UTF-16
> >> >> >> source
> >> >> >> files.
> >> >> >>
> >> >> >> Modified: cfe/trunk/test/Preprocessor/pragma_sysheader.c
> >> >> >> URL:
> >> >> >>
> >> >> >>
> >> >> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/pragma_sysheader.c?rev=154776&r1=154775&r2=154776&view=diff
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >>
> ==============================================================================
> >> >> >> --- cfe/trunk/test/Preprocessor/pragma_sysheader.c (original)
> >> >> >> +++ cfe/trunk/test/Preprocessor/pragma_sysheader.c Sun Apr 15
> >> >> >> 17:09:44
> >> >> >> 2012
> >> >> >> @@ -1,4 +1,4 @@
> >> >> >> -// RUN: %clang -verify -pedantic %s -fsyntax-only
> >> >> >> +// RUN: %clang_cc1 -verify -pedantic %s -fsyntax-only
> >> >> >>  // RUN: %clang_cc1 -E %s | FileCheck %s
> >> >> >>  // rdar://6899937
> >> >> >>  #include "pragma_sysheader.h"
> >> >> >>
> >> >> >> Modified: cfe/trunk/test/Sema/surpress-deprecated.c
> >> >> >> URL:
> >> >> >>
> >> >> >>
> >> >> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/surpress-deprecated.c?rev=154776&r1=154775&r2=154776&view=diff
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >>
> ==============================================================================
> >> >> >> --- cfe/trunk/test/Sema/surpress-deprecated.c (original)
> >> >> >> +++ cfe/trunk/test/Sema/surpress-deprecated.c Sun Apr 15 17:09:44
> >> >> >> 2012
> >> >> >> @@ -1,4 +1,4 @@
> >> >> >> -// RUN: %clang -fsyntax-only -Wno-deprecated-declarations -verify
> >> >> >> %s
> >> >> >> +// RUN: %clang_cc1 -fsyntax-only -Wno-deprecated-declarations
> >> >> >> -verify
> >> >> >> %s
> >> >> >>  extern void OldFunction() __attribute__((deprecated));
> >> >> >>
> >> >> >>  int main (int argc, const char * argv[]) {
> >> >> >>
> >> >> >> Modified: cfe/trunk/test/SemaCXX/warn-everthing.cpp
> >> >> >> URL:
> >> >> >>
> >> >> >>
> >> >> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-everthing.cpp?rev=154776&r1=154775&r2=154776&view=diff
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >>
> ==============================================================================
> >> >> >> --- cfe/trunk/test/SemaCXX/warn-everthing.cpp (original)
> >> >> >> +++ cfe/trunk/test/SemaCXX/warn-everthing.cpp Sun Apr 15 17:09:44
> >> >> >> 2012
> >> >> >> @@ -1,4 +1,4 @@
> >> >> >> -// RUN: %clang -Weverything -fsyntax-only %s -verify
> >> >> >> +// RUN: %clang_cc1 -Weverything -fsyntax-only %s -verify
> >> >> >>
> >> >> >>  // This previously crashed due to a bug in the CFG.  Exercising
> all
> >> >> >>  // warnings helps check CFG construction.
> >> >> >> @@ -8,6 +8,6 @@
> >> >> >>   ~PR12271();
> >> >> >>  };
> >> >> >>
> >> >> >> -void testPR12271() {
> >> >> >> -  PR12271 a[1][1];
> >> >> >> -}
> >> >> >> \ No newline at end of file
> >> >> >> +void testPR12271() { // expected-warning {{no previous prototype
> >> >> >> for
> >> >> >> function 'testPR12271'}}
> >> >> >> +  PR12271 a[1][1]; // expected-warning {{unused variable 'a'}}
> >> >> >> +}
> >> >> >>
> >> >> >>
> >> >> >> _______________________________________________
> >> >> >> cfe-commits mailing list
> >> >> >> cfe-commits at cs.uiuc.edu
> >> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >> >> >
> >> >> >
> >> >> > The only way I would see lit actually reacting would be to
> terminate
> >> >> > the
> >> >> > processus with a non-0 result, which should be reported.
> >> >>
> >> >> Right
> >> >>
> >> >> > Perhaps that we could add a "developer" option provoking this kind
> of
> >> >> > behavior for unknown flags and have lit expand %clang and
> %clang_cc1
> >> >> > automatically with this flag. This flag should then be negated in
> the
> >> >> > few
> >> >> > tests that actually test the driver diagnostic.
> >> >>
> >> >> Ideally, I'm wondering whether it makes sense to have even release
> >> >> versions of clang produce a non-zero exit code for completely unknown
> >> >> flags. If someone passes -foobar to clang, should we really silently
> >> >> (wrt exit code, at least) succeed?
> >> >
> >> >
> >> > What if the flag is a GCC flag that "might" work depending on what CC
> is
> >> > set
> >> > to?
> >>
> >> Are there particular cases of this that you're aware? For what it's
> >> worth, GCC errors on unknown flags, which seems helpful. I could
> >> imagine there being certain subcategories (-f*, -g*, etc) that we
> >> could handle differently (either explicitly downgrading to a warning
> >> for them, or whatever else seemed appropriate)
> >
> >
> > Yes, there are loads of GCC flags that Clang doesn't support yet, even
> just
> > to drop them on the floor.
> >
> > People expect Clang to be (essentially) a drop-in replacement for GCC.
> They
> > don't expect the reverse (generally), although the fact that the reverse
> is
> > not true has bitten us repeatedly already. I don't think we can justify
> > Clang's behavior on GCC's here.
> >
> >>
> >>
> >> Actually, on further experimentation I don't know what GCC is doing.
> >> -verify, -bar, -baz produce "unrecognized option" but don't fail.
> >> "-foo" produces "cc1plus: error: unrecognized command line option
> >> "-foo"" and fails.
> >
> >
> > "-foo" is special -- it thinks it's an argument to cc1plus (-f... is
> > generally an argument to cc1plus) which has a stricter behavior.
> >
> >>
> >>
> >> > I think we need to let users ask for either warnings or errors about
> >> > unknown flags, just as we do today.
> >>
> >> So I understand you clearly - by this are you suggesting "no change",
> >> or some kind of change that would still preserve that behavior?
> >
> >
> > No change. Maybe I'm just not sufficiently paranoid, but this just
> doesn't
> > seem like a big enough deal to expend this much effort to fix
> permanently.
>
> Fair enough - if I were really paranoid I would've pushed for the
> comprehensive solution rather than just fixing the problems I could
> find - but I figured I'd mention it in the CR in case anyone had some
> good ideas/discussion.
>
> > If you want to fix it, maybe introduce a set of warnings to lit.py? I
> > dunno... I'm not sure what the correct fix is, but I don't think it is to
> > make the user-facing Clang driver produce more errors or support more
> flags.
> > This is a problem for developers, and we should shield any solutions to
> it
> > from the user-facing interfaces.
>
> The immediate issue is for developers, but I just thought this might
> be also biting users without them realizing it (as we didn't) when
> passing flags that were being ignored just because it was a typo.
> Perhaps I'm mistaken.
>
> - David
>

I was just thinking that if clang warns about this then maybe it should be
possible to change lit so that it intercepts the warnings (output
redirection) and then with a couple of regex matching checking for the
presence of some specific warnings (like missing options) should be easily
possible.

This way we would not be further modifying clang to accommodate tests but
instead modify the test infrastructure itself.

-- Matthieu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120417/7aa81564/attachment.html>