r291963 - [clang] Emit `diagnose_if` warnings from system headers

[Richard Smith via cfe-commits]

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 <hans at chromium.org> wrote:

> Ping?
>
> On Thu, Jan 26, 2017 at 10:21 AM, Hans Wennborg <hans at chromium.org> wrote:
> > Ping?
> >
> > On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg <hans at chromium.org>
> wrote:
> >> Ping?
> >>
> >> On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg <hans at chromium.org>
> wrote:
> >>> Richard, what do you think?
> >>>
> >>> On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier <eric at 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.iv at 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 at 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 at lists.llvm.org
> >>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >>>>>
> >>>>>
> >>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170131/ce883107/attachment.html>