<br><br><div class="gmail_quote">Le 16 avril 2012 20:09, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>></span> a écrit :<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Mon, Apr 16, 2012 at 11:03 AM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:<br>
> On Mon, Apr 16, 2012 at 7:53 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>> On Mon, Apr 16, 2012 at 10:43 AM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>><br>
>> wrote:<br>
>> > On Mon, Apr 16, 2012 at 7:38 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> On Mon, Apr 16, 2012 at 10:29 AM, Matthieu Monrocq<br>
>> >> <<a href="mailto:matthieu.monrocq@gmail.com">matthieu.monrocq@gmail.com</a>> wrote:<br>
>> >> ><br>
>> >> ><br>
>> >> > Le 16 avril 2012 00:09, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> a écrit :<br>
>> >> ><br>
>> >> >> Author: dblaikie<br>
>> >> >> Date: Sun Apr 15 17:09:44 2012<br>
>> >> >> New Revision: 154776<br>
>> >> >><br>
>> >> >> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=154776&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=154776&view=rev</a><br>
>> >> >> Log:<br>
>> >> >> Fix tests that weren't actually verifying anything.<br>
>> >> >><br>
>> >> >> Passing -verify to clang without -cc1 or -Xclang silently passes<br>
>> >> >> (with<br>
>> >> >> a<br>
>> >> >> printed warning, but lit doesn't care about that). This change adds<br>
>> >> >> -cc1<br>
>> >> >> or,<br>
>> >> >> as is necessary in one case, -Xclang to fix this so that these tests<br>
>> >> >> are<br>
>> >> >> actually verifying as intended.<br>
>> >> >><br>
>> >> >> I'd like to change the driver so this kind of mistake could not be<br>
>> >> >> made,<br>
>> >> >> but<br>
>> >> >> I'm not entirely sure how. Further, since the driver only warns<br>
>> >> >> about<br>
>> >> >> unknown<br>
>> >> >> flags in general, we could have similar bugs with a misspellings of<br>
>> >> >> arguments<br>
>> >> >> that would be nice to find.<br>
>> >> >><br>
>> >> >> Modified:<br>
>> >> >>    cfe/trunk/test/Analysis/objc-bool.m<br>
>> >> >>    cfe/trunk/test/Headers/typedef_guards.c<br>
>> >> >>    cfe/trunk/test/Lexer/utf-16.c<br>
>> >> >>    cfe/trunk/test/Preprocessor/pragma_sysheader.c<br>
>> >> >>    cfe/trunk/test/Sema/surpress-deprecated.c<br>
>> >> >>    cfe/trunk/test/SemaCXX/warn-everthing.cpp<br>
>> >> >><br>
>> >> >> Modified: cfe/trunk/test/Analysis/objc-bool.m<br>
>> >> >> URL:<br>
>> >> >><br>
>> >> >><br>
>> >> >> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc-bool.m?rev=154776&r1=154775&r2=154776&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc-bool.m?rev=154776&r1=154775&r2=154776&view=diff</a><br>

>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >> ==============================================================================<br>
>> >> >> --- cfe/trunk/test/Analysis/objc-bool.m (original)<br>
>> >> >> +++ cfe/trunk/test/Analysis/objc-bool.m Sun Apr 15 17:09:44 2012<br>
>> >> >> @@ -1,4 +1,4 @@<br>
>> >> >> -// RUN: %clang --analyze %s -o %t -verify<br>
>> >> >> +// RUN: %clang --analyze %s -o %t -Xclang -verify<br>
>> >> >><br>
>> >> >>  // Test handling of ObjC bool literals.<br>
>> >> >><br>
>> >> >><br>
>> >> >> Modified: cfe/trunk/test/Headers/typedef_guards.c<br>
>> >> >> URL:<br>
>> >> >><br>
>> >> >><br>
>> >> >> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/typedef_guards.c?rev=154776&r1=154775&r2=154776&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/typedef_guards.c?rev=154776&r1=154775&r2=154776&view=diff</a><br>

>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >> ==============================================================================<br>
>> >> >> --- cfe/trunk/test/Headers/typedef_guards.c (original)<br>
>> >> >> +++ cfe/trunk/test/Headers/typedef_guards.c Sun Apr 15 17:09:44 2012<br>
>> >> >> @@ -1,4 +1,4 @@<br>
>> >> >> -// RUN: %clang -fsyntax-only -verify %s<br>
>> >> >> +// RUN: %clang_cc1 -fsyntax-only -verify %s<br>
>> >> >><br>
>> >> >>  // NULL is rdefined in stddef.h<br>
>> >> >>  #define NULL ((void*) 0)<br>
>> >> >><br>
>> >> >> Modified: cfe/trunk/test/Lexer/utf-16.c<br>
>> >> >> URL:<br>
>> >> >><br>
>> >> >><br>
>> >> >> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/utf-16.c?rev=154776&r1=154775&r2=154776&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/utf-16.c?rev=154776&r1=154775&r2=154776&view=diff</a><br>

>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >> ==============================================================================<br>
>> >> >> --- cfe/trunk/test/Lexer/utf-16.c (original)<br>
>> >> >> +++ cfe/trunk/test/Lexer/utf-16.c Sun Apr 15 17:09:44 2012<br>
>> >> >> @@ -1,4 +1,4 @@<br>
>> >> >> -// RUN: not %clang %s -fsyntax-only -verify<br>
>> >> >> +// RUN: %clang_cc1 %s -fsyntax-only -verify<br>
>> >> >>  // rdar://7876588<br>
>> >> >><br>
>> >> >>  // This test verifies that clang gives a decent error for UTF-16<br>
>> >> >> source<br>
>> >> >> files.<br>
>> >> >><br>
>> >> >> Modified: cfe/trunk/test/Preprocessor/pragma_sysheader.c<br>
>> >> >> URL:<br>
>> >> >><br>
>> >> >><br>
>> >> >> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/pragma_sysheader.c?rev=154776&r1=154775&r2=154776&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/pragma_sysheader.c?rev=154776&r1=154775&r2=154776&view=diff</a><br>

>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >> ==============================================================================<br>
>> >> >> --- cfe/trunk/test/Preprocessor/pragma_sysheader.c (original)<br>
>> >> >> +++ cfe/trunk/test/Preprocessor/pragma_sysheader.c Sun Apr 15<br>
>> >> >> 17:09:44<br>
>> >> >> 2012<br>
>> >> >> @@ -1,4 +1,4 @@<br>
>> >> >> -// RUN: %clang -verify -pedantic %s -fsyntax-only<br>
>> >> >> +// RUN: %clang_cc1 -verify -pedantic %s -fsyntax-only<br>
>> >> >>  // RUN: %clang_cc1 -E %s | FileCheck %s<br>
>> >> >>  // rdar://6899937<br>
>> >> >>  #include "pragma_sysheader.h"<br>
>> >> >><br>
>> >> >> Modified: cfe/trunk/test/Sema/surpress-deprecated.c<br>
>> >> >> URL:<br>
>> >> >><br>
>> >> >><br>
>> >> >> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/surpress-deprecated.c?rev=154776&r1=154775&r2=154776&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/surpress-deprecated.c?rev=154776&r1=154775&r2=154776&view=diff</a><br>

>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >> ==============================================================================<br>
>> >> >> --- cfe/trunk/test/Sema/surpress-deprecated.c (original)<br>
>> >> >> +++ cfe/trunk/test/Sema/surpress-deprecated.c Sun Apr 15 17:09:44<br>
>> >> >> 2012<br>
>> >> >> @@ -1,4 +1,4 @@<br>
>> >> >> -// RUN: %clang -fsyntax-only -Wno-deprecated-declarations -verify<br>
>> >> >> %s<br>
>> >> >> +// RUN: %clang_cc1 -fsyntax-only -Wno-deprecated-declarations<br>
>> >> >> -verify<br>
>> >> >> %s<br>
>> >> >>  extern void OldFunction() __attribute__((deprecated));<br>
>> >> >><br>
>> >> >>  int main (int argc, const char * argv[]) {<br>
>> >> >><br>
>> >> >> Modified: cfe/trunk/test/SemaCXX/warn-everthing.cpp<br>
>> >> >> URL:<br>
>> >> >><br>
>> >> >><br>
>> >> >> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-everthing.cpp?rev=154776&r1=154775&r2=154776&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-everthing.cpp?rev=154776&r1=154775&r2=154776&view=diff</a><br>

>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >> ==============================================================================<br>
>> >> >> --- cfe/trunk/test/SemaCXX/warn-everthing.cpp (original)<br>
>> >> >> +++ cfe/trunk/test/SemaCXX/warn-everthing.cpp Sun Apr 15 17:09:44<br>
>> >> >> 2012<br>
>> >> >> @@ -1,4 +1,4 @@<br>
>> >> >> -// RUN: %clang -Weverything -fsyntax-only %s -verify<br>
>> >> >> +// RUN: %clang_cc1 -Weverything -fsyntax-only %s -verify<br>
>> >> >><br>
>> >> >>  // This previously crashed due to a bug in the CFG.  Exercising all<br>
>> >> >>  // warnings helps check CFG construction.<br>
>> >> >> @@ -8,6 +8,6 @@<br>
>> >> >>   ~PR12271();<br>
>> >> >>  };<br>
>> >> >><br>
>> >> >> -void testPR12271() {<br>
>> >> >> -  PR12271 a[1][1];<br>
>> >> >> -}<br>
>> >> >> \ No newline at end of file<br>
>> >> >> +void testPR12271() { // expected-warning {{no previous prototype<br>
>> >> >> for<br>
>> >> >> function 'testPR12271'}}<br>
>> >> >> +  PR12271 a[1][1]; // expected-warning {{unused variable 'a'}}<br>
>> >> >> +}<br>
>> >> >><br>
>> >> >><br>
>> >> >> _______________________________________________<br>
>> >> >> cfe-commits mailing list<br>
>> >> >> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> >> >> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>> >> ><br>
>> >> ><br>
>> >> > The only way I would see lit actually reacting would be to terminate<br>
>> >> > the<br>
>> >> > processus with a non-0 result, which should be reported.<br>
>> >><br>
>> >> Right<br>
>> >><br>
>> >> > Perhaps that we could add a "developer" option provoking this kind of<br>
>> >> > behavior for unknown flags and have lit expand %clang and %clang_cc1<br>
>> >> > automatically with this flag. This flag should then be negated in the<br>
>> >> > few<br>
>> >> > tests that actually test the driver diagnostic.<br>
>> >><br>
>> >> Ideally, I'm wondering whether it makes sense to have even release<br>
>> >> versions of clang produce a non-zero exit code for completely unknown<br>
>> >> flags. If someone passes -foobar to clang, should we really silently<br>
>> >> (wrt exit code, at least) succeed?<br>
>> ><br>
>> ><br>
>> > What if the flag is a GCC flag that "might" work depending on what CC is<br>
>> > set<br>
>> > to?<br>
>><br>
>> Are there particular cases of this that you're aware? For what it's<br>
>> worth, GCC errors on unknown flags, which seems helpful. I could<br>
>> imagine there being certain subcategories (-f*, -g*, etc) that we<br>
>> could handle differently (either explicitly downgrading to a warning<br>
>> for them, or whatever else seemed appropriate)<br>
><br>
><br>
> Yes, there are loads of GCC flags that Clang doesn't support yet, even just<br>
> to drop them on the floor.<br>
><br>
> People expect Clang to be (essentially) a drop-in replacement for GCC. They<br>
> don't expect the reverse (generally), although the fact that the reverse is<br>
> not true has bitten us repeatedly already. I don't think we can justify<br>
> Clang's behavior on GCC's here.<br>
><br>
>><br>
>><br>
>> Actually, on further experimentation I don't know what GCC is doing.<br>
>> -verify, -bar, -baz produce "unrecognized option" but don't fail.<br>
>> "-foo" produces "cc1plus: error: unrecognized command line option<br>
>> "-foo"" and fails.<br>
><br>
><br>
> "-foo" is special -- it thinks it's an argument to cc1plus (-f... is<br>
> generally an argument to cc1plus) which has a stricter behavior.<br>
><br>
>><br>
>><br>
>> > I think we need to let users ask for either warnings or errors about<br>
>> > unknown flags, just as we do today.<br>
>><br>
>> So I understand you clearly - by this are you suggesting "no change",<br>
>> or some kind of change that would still preserve that behavior?<br>
><br>
><br>
> No change. Maybe I'm just not sufficiently paranoid, but this just doesn't<br>
> seem like a big enough deal to expend this much effort to fix permanently.<br>
<br>
</div></div>Fair enough - if I were really paranoid I would've pushed for the<br>
comprehensive solution rather than just fixing the problems I could<br>
find - but I figured I'd mention it in the CR in case anyone had some<br>
good ideas/discussion.<br>
<div class="im"><br>
> If you want to fix it, maybe introduce a set of warnings to lit.py? I<br>
> dunno... I'm not sure what the correct fix is, but I don't think it is to<br>
> make the user-facing Clang driver produce more errors or support more flags.<br>
> This is a problem for developers, and we should shield any solutions to it<br>
> from the user-facing interfaces.<br>
<br>
</div>The immediate issue is for developers, but I just thought this might<br>
be also biting users without them realizing it (as we didn't) when<br>
passing flags that were being ignored just because it was a typo.<br>
Perhaps I'm mistaken.<br>
<span class="HOEnZb"><font color="#888888"><br>
- David<br>
</font></span></blockquote></div><br>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.<br>
<br>This way we would not be further modifying clang to accommodate tests but instead modify the test infrastructure itself.<br><br>-- Matthieu<br>