Awesome. Thank you all! :) On Wed, Feb 1, 2017 at 9:25 AM, Hans Wennborg <h...@chromium.org> wrote:
> Merged this (r291963) in r293783. > > And the others (r293360 + r293369) in r293784. > > Thanks, > Hans > > On Tue, Jan 31, 2017 at 7:17 PM, Richard Smith <richardsm...@google.com> > wrote: > > I'm fine with these patches being merged. Hopefully we still have plenty > of > > time to shake out any problems between now and the release. > > > > On 31 January 2017 at 19:09, George Burgess IV < > george.burgess...@gmail.com> > > wrote: > >> > >> > IIUC the major risk is that diagnose_if itself turns out to be broken, > >> > not that we'd miscompile anything? > >> > >> Correct. These patches should be NFC to code that doesn't use > diagnose_if. > >> > >> If something about that patch *had* to break existing > >> non-diagnose_if-aware code, we're now calling Sema::CheckFunctionCall > (which > >> checks diagnose_if attributes, checks we're not passing a nullptr into > >> _Nonnull args, ...) for more kinds of C++ function calls than before. We > >> weren't calling it before in these places as an optimization: in > theory, it > >> wasn't possible for CheckFunctionCall to find issues in calls to things > like > >> conversion functions. Now that we have diagnose_if, it is. > >> > >> On Tue, Jan 31, 2017 at 6:31 PM, Hans Wennborg <h...@chromium.org> > wrote: > >>> > >>> I'm Ok with taking the larger patch (r293360 + r293369) too. It's been > >>> in tree for a bit, there is still a number of weeks before the > >>> release, and IIUC the major risk is that diagnose_if itself turns out > >>> to be broken, not that we'd miscompile anything? > >>> > >>> 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/di > agnose-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_ob > ject_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/dia > gnose-if-warn-system-header.h > >>> >> >>>>>> (added) > >>> >> >>>>>> +++ cfe/trunk/test/Sema/Inputs/dia > gnose-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
