<div dir="ltr">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.<br><div class="gmail_extra"><br><div class="gmail_quote">On 31 January 2017 at 10:36, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Ping?<br>
<br>
On Thu, Jan 26, 2017 at 10:21 AM, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br>
> Ping?<br>
><br>
> On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br>
>> Ping?<br>
>><br>
>> On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br>
>>> Richard, what do you think?<br>
>>><br>
>>> On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier <<a href="mailto:eric@efcs.ca">eric@efcs.ca</a>> wrote:<br>
>>>> I would love to see this merged. It would make it easier to write libc++<br>
>>>> tests if the tests didn't have to worry about the old 4.0 behavior.<br>
>>>><br>
>>>> CC'ing Richard: Would merging this be OK?<br>
>>>><br>
>>>> On Fri, Jan 13, 2017 at 3:46 PM, George Burgess IV<br>
>>>> <<a href="mailto:george.burgess.iv@gmail.com">george.burgess.iv@gmail.com</a>> wrote:<br>
>>>>><br>
>>>>> Do we want to consider merging this into the release branch? Seems like<br>
>>>>> more of a bugfix than a feature to me.<br>
>>>>><br>
>>>>> On Fri, Jan 13, 2017 at 2:11 PM, Eric Fiselier via cfe-commits<br>
>>>>> <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
>>>>>><br>
>>>>>> Author: ericwf<br>
>>>>>> Date: Fri Jan 13 16:11:40 2017<br>
>>>>>> New Revision: 291963<br>
>>>>>><br>
>>>>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=291963&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=291963&view=rev</a><br>
>>>>>> Log:<br>
>>>>>> [clang] Emit `diagnose_if` warnings from system headers<br>
>>>>>><br>
>>>>>> Summary: In order for libc++ to meaningfully use `diagnose_if` warnings<br>
>>>>>> they need to be emitted from system headers by default. This patch changes<br>
>>>>>> the `diagnose_if` warning diagnostic to be shown in system headers.<br>
>>>>>><br>
>>>>>> Reviewers: george.burgess.iv, rsmith, aaron.ballman<br>
>>>>>><br>
>>>>>> Subscribers: cfe-commits<br>
>>>>>><br>
>>>>>> Differential Revision: <a href="https://reviews.llvm.org/D28703" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D28703</a><br>
>>>>>><br>
>>>>>> Added:<br>
>>>>>>     cfe/trunk/test/Sema/Inputs/<wbr>diagnose-if-warn-system-<wbr>header.h<br>
>>>>>> Modified:<br>
>>>>>>     cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td<br>
>>>>>>     cfe/trunk/test/Sema/diagnose_<wbr>if.c<br>
>>>>>><br>
>>>>>> Modified: cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td<br>
>>>>>> URL:<br>
>>>>>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=291963&r1=291962&r2=291963&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/Basic/<wbr>DiagnosticSemaKinds.td?rev=<wbr>291963&r1=291962&r2=291963&<wbr>view=diff</a><br>
>>>>>><br>
>>>>>> ==============================<wbr>==============================<wbr>==================<br>
>>>>>> --- cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td (original)<br>
>>>>>> +++ cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td Fri Jan 13<br>
>>>>>> 16:11:40 2017<br>
>>>>>> @@ -3380,7 +3380,8 @@ def note_ovl_candidate_has_pass_<wbr>object_s<br>
>>>>>>      "candidate address cannot be taken because parameter %0 has "<br>
>>>>>>      "pass_object_size attribute">;<br>
>>>>>>  def err_diagnose_if_succeeded : Error<"%0">;<br>
>>>>>> -def warn_diagnose_if_succeeded : Warning<"%0">,<br>
>>>>>> InGroup<UserDefinedWarnings>;<br>
>>>>>> +def warn_diagnose_if_succeeded : Warning<"%0">,<br>
>>>>>> InGroup<UserDefinedWarnings>,<br>
>>>>>> +    ShowInSystemHeader;<br>
>>>>>>  def note_ovl_candidate_disabled_<wbr>by_function_cond_attr : Note<<br>
>>>>>>      "candidate disabled: %0">;<br>
>>>>>>  def note_ovl_candidate_disabled_<wbr>by_extension : Note<<br>
>>>>>><br>
>>>>>> Added: cfe/trunk/test/Sema/Inputs/<wbr>diagnose-if-warn-system-<wbr>header.h<br>
>>>>>> URL:<br>
>>>>>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h?rev=291963&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/Sema/<wbr>Inputs/diagnose-if-warn-<wbr>system-header.h?rev=291963&<wbr>view=auto</a><br>
>>>>>><br>
>>>>>> ==============================<wbr>==============================<wbr>==================<br>
>>>>>> --- cfe/trunk/test/Sema/Inputs/<wbr>diagnose-if-warn-system-<wbr>header.h (added)<br>
>>>>>> +++ cfe/trunk/test/Sema/Inputs/<wbr>diagnose-if-warn-system-<wbr>header.h Fri Jan<br>
>>>>>> 13 16:11:40 2017<br>
>>>>>> @@ -0,0 +1,11 @@<br>
>>>>>> +#pragma GCC system_header<br>
>>>>>> +<br>
>>>>>> +inline int system_header_func(int x)<br>
>>>>>> +  __attribute__((diagnose_if(x == x, "system header warning",<br>
>>>>>> "warning"))) // expected-note {{from 'diagnose_if' attribute}}<br>
>>>>>> +{<br>
>>>>>> +  return 0;<br>
>>>>>> +}<br>
>>>>>> +<br>
>>>>>> +void test_system_header() {<br>
>>>>>> +  system_header_func(0); // expected-warning {{system header warning}}<br>
>>>>>> +}<br>
>>>>>><br>
>>>>>> Modified: cfe/trunk/test/Sema/diagnose_<wbr>if.c<br>
>>>>>> URL:<br>
>>>>>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/diagnose_if.c?rev=291963&r1=291962&r2=291963&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/Sema/<wbr>diagnose_if.c?rev=291963&r1=<wbr>291962&r2=291963&view=diff</a><br>
>>>>>><br>
>>>>>> ==============================<wbr>==============================<wbr>==================<br>
>>>>>> --- cfe/trunk/test/Sema/diagnose_<wbr>if.c (original)<br>
>>>>>> +++ cfe/trunk/test/Sema/diagnose_<wbr>if.c Fri Jan 13 16:11:40 2017<br>
>>>>>> @@ -150,3 +150,6 @@ void alwaysWarnWithArg(int a) _diagnose_<br>
>>>>>>  void runAlwaysWarnWithArg(int a) {<br>
>>>>>>    alwaysWarnWithArg(a); // expected-warning{{alwaysWarn}}<br>
>>>>>>  }<br>
>>>>>> +<br>
>>>>>> +// Test that diagnose_if warnings generated in system headers are not<br>
>>>>>> ignored.<br>
>>>>>> +#include "Inputs/diagnose-if-warn-<wbr>system-header.h"<br>
>>>>>><br>
>>>>>><br>
>>>>>> ______________________________<wbr>_________________<br>
>>>>>> cfe-commits mailing list<br>
>>>>>> <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
>>>>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
>>>>><br>
>>>>><br>
>>>><br>
</blockquote></div><br></div></div>