[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

[David Blaikie]

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)

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.

> 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?

I don't think anything that keeps unknown flags as warnings by default
will protect from the kinds of mistakes I mentioned. I suppose special
casing for "-verify" could work (what about when some other compiler
decides to use that flag (indeed, even Clang already has a -verify
flag (something echristo added for verifying debug information on
darwin, as far as I understand it))) but wouldn't account for
misspellings, though I haven't seen (or tried to look - they would be
harder to find) for any of those yet.

- David