Re: r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings

2016-10-11 Thread Bruno Cardoso Lopes via cfe-commits
Let's see how it goes: Committed r283913

On Tue, Oct 11, 2016 at 11:13 AM, Bruno Cardoso Lopes
 wrote:
> On Tue, Oct 11, 2016 at 10:09 AM, Renato Golin  
> wrote:
>> On 11 October 2016 at 16:34, Bruno Cardoso Lopes
>>  wrote:
>>> Thanks Renato!
>>
>> So, Daniel Jasper did a trick on r283853 (clang || true) to make it
>> not fail when it returns on error. However, I wasn't able to make it
>> return anything but 0, so I'm at odds why this fails on the bot.
>>
>> I was expecting it to actually return non-zero, since it emits an
>> error, which is even weirder. So, using "not clang" doesn't work in
>> this case. Do you have any ideas why this behaviour is like that?
>
> I have no idea. I'm gonna try to re-introduce the commit without a
> target specifc triple, that hopefully should be enough to get it
> working (there was no specific reason this test should be using a
> specific target anyway)
>
> Thanks again Renato
>
>
>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings

2016-10-11 Thread Bruno Cardoso Lopes via cfe-commits
On Tue, Oct 11, 2016 at 10:09 AM, Renato Golin  wrote:
> On 11 October 2016 at 16:34, Bruno Cardoso Lopes
>  wrote:
>> Thanks Renato!
>
> So, Daniel Jasper did a trick on r283853 (clang || true) to make it
> not fail when it returns on error. However, I wasn't able to make it
> return anything but 0, so I'm at odds why this fails on the bot.
>
> I was expecting it to actually return non-zero, since it emits an
> error, which is even weirder. So, using "not clang" doesn't work in
> this case. Do you have any ideas why this behaviour is like that?

I have no idea. I'm gonna try to re-introduce the commit without a
target specifc triple, that hopefully should be enough to get it
working (there was no specific reason this test should be using a
specific target anyway)

Thanks again Renato



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings

2016-10-11 Thread Renato Golin via cfe-commits
On 11 October 2016 at 16:34, Bruno Cardoso Lopes
 wrote:
> Thanks Renato!

So, Daniel Jasper did a trick on r283853 (clang || true) to make it
not fail when it returns on error. However, I wasn't able to make it
return anything but 0, so I'm at odds why this fails on the bot.

I was expecting it to actually return non-zero, since it emits an
error, which is even weirder. So, using "not clang" doesn't work in
this case. Do you have any ideas why this behaviour is like that?

cheers,
--renato
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings

2016-10-11 Thread Bruno Cardoso Lopes via cfe-commits
Thanks Renato!

On Tue, Oct 11, 2016 at 3:36 AM, Renato Golin  wrote:
> On 11 October 2016 at 10:14, Renato Golin  wrote:
>> clang-4.0: warning: no such sysroot directory: '/FOO' [-Wmissing-sysroot]
>> error: unable to create target: 'No available targets are compatible
>> with this triple.'
>> 1 error generated.
>
> Reverted in r283868, as it was also breaking ARM bots.
>
> cheers,
> --renato



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings

2016-10-11 Thread Renato Golin via cfe-commits
On 11 October 2016 at 10:14, Renato Golin  wrote:
> clang-4.0: warning: no such sysroot directory: '/FOO' [-Wmissing-sysroot]
> error: unable to create target: 'No available targets are compatible
> with this triple.'
> 1 error generated.

Reverted in r283868, as it was also breaking ARM bots.

cheers,
--renato
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings

2016-10-11 Thread Renato Golin via cfe-commits
On 11 October 2016 at 01:01, Bruno Cardoso Lopes via cfe-commits
 wrote:
> Author: bruno
> Date: Mon Oct 10 19:01:22 2016
> New Revision: 283827
>
> URL: http://llvm.org/viewvc/llvm-project?rev=283827&view=rev
> Log:
> [Driver][Diagnostics] Make 'show option names' default for driver warnings
>
> Currently, driver level warnings do not show option names (e.g. warning:
> complain about foo [-Woption-name]) in a diagnostic unless
> -fdiagnostics-show-option is explictly specified. OTOH, the driver by
> default turn this option on for CC1. Change the logic to show option
> names by default in the driver as well.
>
> Differential Revision: https://reviews.llvm.org/D24516

Hi Bruno,

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/3292

clang-4.0: warning: no such sysroot directory: '/FOO' [-Wmissing-sysroot]
error: unable to create target: 'No available targets are compatible
with this triple.'
1 error generated.

cheers,
--renato
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings

2016-10-10 Thread Bruno Cardoso Lopes via cfe-commits
Author: bruno
Date: Mon Oct 10 19:01:22 2016
New Revision: 283827

URL: http://llvm.org/viewvc/llvm-project?rev=283827&view=rev
Log:
[Driver][Diagnostics] Make 'show option names' default for driver warnings

Currently, driver level warnings do not show option names (e.g. warning:
complain about foo [-Woption-name]) in a diagnostic unless
-fdiagnostics-show-option is explictly specified. OTOH, the driver by
default turn this option on for CC1. Change the logic to show option
names by default in the driver as well.

Differential Revision: https://reviews.llvm.org/D24516

rdar://problem/27300909

Added:
cfe/trunk/test/Driver/show-option-names.c
Modified:
cfe/trunk/include/clang/Frontend/CompilerInvocation.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Frontend/CompilerInvocation.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInvocation.h?rev=283827&r1=283826&r2=283827&view=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInvocation.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInvocation.h Mon Oct 10 19:01:22 
2016
@@ -48,7 +48,8 @@ class DiagnosticsEngine;
 /// report the error(s).
 bool ParseDiagnosticArgs(DiagnosticOptions &Opts, llvm::opt::ArgList &Args,
  DiagnosticsEngine *Diags = nullptr,
- bool DefaultDiagColor = true);
+ bool DefaultDiagColor = true,
+ bool DefaultShowOpt = true);
 
 class CompilerInvocationBase : public RefCountedBase {
   void operator=(const CompilerInvocationBase &) = delete;

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=283827&r1=283826&r2=283827&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Mon Oct 10 19:01:22 2016
@@ -953,7 +953,7 @@ static bool parseShowColorsArgs(const Ar
 
 bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
 DiagnosticsEngine *Diags,
-bool DefaultDiagColor) {
+bool DefaultDiagColor, bool DefaultShowOpt) {
   using namespace options;
   bool Success = true;
 
@@ -973,7 +973,9 @@ bool clang::ParseDiagnosticArgs(Diagnost
   Opts.ShowFixits = !Args.hasArg(OPT_fno_diagnostics_fixit_info);
   Opts.ShowLocation = !Args.hasArg(OPT_fno_show_source_location);
   Opts.AbsolutePath = Args.hasArg(OPT_fdiagnostics_absolute_paths);
-  Opts.ShowOptionNames = Args.hasArg(OPT_fdiagnostics_show_option);
+  Opts.ShowOptionNames =
+  Args.hasFlag(OPT_fdiagnostics_show_option,
+   OPT_fno_diagnostics_show_option, DefaultShowOpt);
 
   llvm::sys::Process::UseANSIEscapeCodes(Args.hasArg(OPT_fansi_escape_codes));
 
@@ -2400,8 +2402,9 @@ bool CompilerInvocation::CreateFromArgs(
   Success &= ParseAnalyzerArgs(*Res.getAnalyzerOpts(), Args, Diags);
   Success &= ParseMigratorArgs(Res.getMigratorOpts(), Args);
   ParseDependencyOutputArgs(Res.getDependencyOutputOpts(), Args);
-  Success &= ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
- false /*DefaultDiagColor*/);
+  Success &=
+  ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
+  false /*DefaultDiagColor*/, false 
/*DefaultShowOpt*/);
   ParseCommentArgs(LangOpts.CommentOpts, Args);
   ParseFileSystemArgs(Res.getFileSystemOpts(), Args);
   // FIXME: We shouldn't have to pass the DashX option around here

Added: cfe/trunk/test/Driver/show-option-names.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/show-option-names.c?rev=283827&view=auto
==
--- cfe/trunk/test/Driver/show-option-names.c (added)
+++ cfe/trunk/test/Driver/show-option-names.c Mon Oct 10 19:01:22 2016
@@ -0,0 +1,5 @@
+// RUN: %clang -c -target i386-apple-darwin10 -isysroot /FOO %s 2>&1 | 
FileCheck --check-prefix=CHECK-SHOW-OPTION-NAMES %s
+// CHECK-SHOW-OPTION-NAMES: warning: no such sysroot directory: 
'{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
+
+// RUN: %clang -c -target i386-apple-darwin10 -fno-diagnostics-show-option 
-isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
+// CHECK-NO-SHOW-OPTION-NAMES: warning: no such sysroot directory: 
'{{([A-Za-z]:.*)?}}/FOO'{{$}}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits