Re: r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings
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
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
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
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
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
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
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