> If not, perhaps we should disable the attribute for the Clang 4 release instead
FWIW, I'd strongly prefer to do this over letting diagnose_if go into Clang 4 unpatched. So, if my patch does feel too big, I'm happy to let diagnose_if be a new-in-clang-5 attribute. :) On Tue, Jan 31, 2017 at 11:11 AM, Richard Smith <richardsm...@google.com> wrote: > Yes, this makes sense. We should also decide what we're going to do about > the larger diagnose_if patch that George has asked to be ported to Clang 4. > Are you comfortable taking a patch of that size? If not, perhaps we should > disable the attribute for the Clang 4 release instead. > > > On 31 January 2017 at 10:36, Hans Wennborg <h...@chromium.org> wrote: > >> Ping? >> >> On Thu, Jan 26, 2017 at 10:21 AM, Hans Wennborg <h...@chromium.org> >> wrote: >> > Ping? >> > >> > On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg <h...@chromium.org> >> wrote: >> >> Ping? >> >> >> >> On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg <h...@chromium.org> >> wrote: >> >>> Richard, what do you think? >> >>> >> >>> On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier <e...@efcs.ca> wrote: >> >>>> I would love to see this merged. It would make it easier to write >> libc++ >> >>>> tests if the tests didn't have to worry about the old 4.0 behavior. >> >>>> >> >>>> CC'ing Richard: Would merging this be OK? >> >>>> >> >>>> On Fri, Jan 13, 2017 at 3:46 PM, George Burgess IV >> >>>> <george.burgess...@gmail.com> wrote: >> >>>>> >> >>>>> Do we want to consider merging this into the release branch? Seems >> like >> >>>>> more of a bugfix than a feature to me. >> >>>>> >> >>>>> On Fri, Jan 13, 2017 at 2:11 PM, Eric Fiselier via cfe-commits >> >>>>> <cfe-commits@lists.llvm.org> wrote: >> >>>>>> >> >>>>>> Author: ericwf >> >>>>>> Date: Fri Jan 13 16:11:40 2017 >> >>>>>> New Revision: 291963 >> >>>>>> >> >>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=291963&view=rev >> >>>>>> Log: >> >>>>>> [clang] Emit `diagnose_if` warnings from system headers >> >>>>>> >> >>>>>> Summary: In order for libc++ to meaningfully use `diagnose_if` >> warnings >> >>>>>> they need to be emitted from system headers by default. This patch >> changes >> >>>>>> the `diagnose_if` warning diagnostic to be shown in system headers. >> >>>>>> >> >>>>>> Reviewers: george.burgess.iv, rsmith, aaron.ballman >> >>>>>> >> >>>>>> Subscribers: cfe-commits >> >>>>>> >> >>>>>> Differential Revision: https://reviews.llvm.org/D28703 >> >>>>>> >> >>>>>> Added: >> >>>>>> cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h >> >>>>>> Modified: >> >>>>>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> >>>>>> cfe/trunk/test/Sema/diagnose_if.c >> >>>>>> >> >>>>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> >>>>>> URL: >> >>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >> Basic/DiagnosticSemaKinds.td?rev=291963&r1=291962&r2=291963&view=diff >> >>>>>> >> >>>>>> ============================================================ >> ================== >> >>>>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> (original) >> >>>>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan >> 13 >> >>>>>> 16:11:40 2017 >> >>>>>> @@ -3380,7 +3380,8 @@ def note_ovl_candidate_has_pass_object_s >> >>>>>> "candidate address cannot be taken because parameter %0 has " >> >>>>>> "pass_object_size attribute">; >> >>>>>> def err_diagnose_if_succeeded : Error<"%0">; >> >>>>>> -def warn_diagnose_if_succeeded : Warning<"%0">, >> >>>>>> InGroup<UserDefinedWarnings>; >> >>>>>> +def warn_diagnose_if_succeeded : Warning<"%0">, >> >>>>>> InGroup<UserDefinedWarnings>, >> >>>>>> + ShowInSystemHeader; >> >>>>>> def note_ovl_candidate_disabled_by_function_cond_attr : Note< >> >>>>>> "candidate disabled: %0">; >> >>>>>> def note_ovl_candidate_disabled_by_extension : Note< >> >>>>>> >> >>>>>> Added: cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h >> >>>>>> URL: >> >>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/Inpu >> ts/diagnose-if-warn-system-header.h?rev=291963&view=auto >> >>>>>> >> >>>>>> ============================================================ >> ================== >> >>>>>> --- cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h >> (added) >> >>>>>> +++ cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h >> Fri Jan >> >>>>>> 13 16:11:40 2017 >> >>>>>> @@ -0,0 +1,11 @@ >> >>>>>> +#pragma GCC system_header >> >>>>>> + >> >>>>>> +inline int system_header_func(int x) >> >>>>>> + __attribute__((diagnose_if(x == x, "system header warning", >> >>>>>> "warning"))) // expected-note {{from 'diagnose_if' attribute}} >> >>>>>> +{ >> >>>>>> + return 0; >> >>>>>> +} >> >>>>>> + >> >>>>>> +void test_system_header() { >> >>>>>> + system_header_func(0); // expected-warning {{system header >> warning}} >> >>>>>> +} >> >>>>>> >> >>>>>> Modified: cfe/trunk/test/Sema/diagnose_if.c >> >>>>>> URL: >> >>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/diag >> nose_if.c?rev=291963&r1=291962&r2=291963&view=diff >> >>>>>> >> >>>>>> ============================================================ >> ================== >> >>>>>> --- cfe/trunk/test/Sema/diagnose_if.c (original) >> >>>>>> +++ cfe/trunk/test/Sema/diagnose_if.c Fri Jan 13 16:11:40 2017 >> >>>>>> @@ -150,3 +150,6 @@ void alwaysWarnWithArg(int a) _diagnose_ >> >>>>>> void runAlwaysWarnWithArg(int a) { >> >>>>>> alwaysWarnWithArg(a); // expected-warning{{alwaysWarn}} >> >>>>>> } >> >>>>>> + >> >>>>>> +// Test that diagnose_if warnings generated in system headers are >> not >> >>>>>> ignored. >> >>>>>> +#include "Inputs/diagnose-if-warn-system-header.h" >> >>>>>> >> >>>>>> >> >>>>>> _______________________________________________ >> >>>>>> cfe-commits mailing list >> >>>>>> cfe-commits@lists.llvm.org >> >>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >>>>> >> >>>>> >> >>>> >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
