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

2017-02-01 Thread George Burgess IV via cfe-commits
Awesome. Thank you all! :) On Wed, Feb 1, 2017 at 9:25 AM, Hans Wennborg 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 > wrote: > >

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

2017-02-01 Thread Hans Wennborg via cfe-commits
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 wrote: > I'm fine with these patches being merged. Hopefully we still have plenty of > time to shake out any problems between

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

2017-01-31 Thread Richard Smith via cfe-commits
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 wrote: > > IIUC the major risk is that diagnose_if itself turns out to be broken, >

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

2017-01-31 Thread George Burgess IV via cfe-commits
> 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

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

2017-01-31 Thread Hans Wennborg via cfe-commits
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

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

2017-01-31 Thread George Burgess IV via cfe-commits
> 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

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

2017-01-31 Thread 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

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

2017-01-31 Thread Hans Wennborg via cfe-commits
Ping? On Thu, Jan 26, 2017 at 10:21 AM, Hans Wennborg wrote: > Ping? > > On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg wrote: >> Ping? >> >> On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg wrote: >>> Richard, what do you think? >>> >>>

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

2017-01-26 Thread Hans Wennborg via cfe-commits
Ping? On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg wrote: > Ping? > > On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg wrote: >> Richard, what do you think? >> >> On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier wrote: >>> I would love to see

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

2017-01-23 Thread Hans Wennborg via cfe-commits
Ping? On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg wrote: > Richard, what do you think? > > On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier wrote: >> I would love to see this merged. It would make it easier to write libc++ >> tests if the tests didn't have to

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

2017-01-17 Thread Hans Wennborg via cfe-commits
Richard, what do you think? On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier 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

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

2017-01-13 Thread Eric Fiselier via cfe-commits
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