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/ > Inputs/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/ > diagnose_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
