<div dir="ltr">What Patrik said. =D This LGTM once we're confident that for ToT Clang, we enable the warning. I like the approach taken, it seems much nicer than a version check!</div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Sat, Aug 23, 2014 at 1:06 PM, Patrik Hägglund H <span dir="ltr"><<a href="mailto:patrik.h.hagglund@ericsson.com" target="_blank">patrik.h.hagglund@ericsson.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Dylan,<br>
<br>
I was about to commit the version check Chandler proposed. But, a small test case seems even better. However, for clang-3.5rc2 I still get<br>
<br>
-- Performing Test CXX_WONT_WARN_ON_FINAL_NONVIRTUALDTOR - Failed<br>
<br>
Perhaps you need to add -std=c++11 to the check?<br>
<br>
When fixed, please commit!<br>
<span class="HOEnZb"><font color="#888888"><br>
/Patrik Hägglund<br>
</font></span><div class="im HOEnZb"><br>
-----Original Message-----<br>
From: <a href="mailto:dylan.noblesmith1@maine.edu">dylan.noblesmith1@maine.edu</a> [mailto:<a href="mailto:dylan.noblesmith1@maine.edu">dylan.noblesmith1@maine.edu</a>] On Behalf Of Dylan Noblesmith<br>
Sent: den 23 augusti 2014 20:37<br>
To: Chandler Carruth<br>
Cc: Patrik Hägglund H; <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
Subject: Re: [llvm] r216170 - Make format_object_base's destructor protected and non-virtual.<br>
<br>
</div><div class="HOEnZb"><div class="h5">On Sat, Aug 23, 2014 at 6:09 AM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:<br>
><br>
> On Sat, Aug 23, 2014 at 2:58 AM, Patrik Hägglund H<br>
> <<a href="mailto:patrik.h.hagglund@ericsson.com">patrik.h.hagglund@ericsson.com</a>> wrote:<br>
>><br>
>> > In all seriousness, we need to find a solution to this soon, or revert<br>
>> > this change.<br>
>><br>
>><br>
>><br>
>> +1<br>
>><br>
>><br>
>><br>
>> > For those who want to use old/pre-installed/official release clangs<br>
>> > rather than more recent iterations, they can just disable –Werror (it's not<br>
>> > the default anyway).<br>
>><br>
>><br>
>><br>
>> For us, disabling –Werror is currently not an option. If this is the<br>
>> intention, I think the policy should be updated first. (I would rather like<br>
>> the policy to be strengthened, to support –Werror builds with the latest<br>
>> official release of _both_ clang and gcc.)<br>
><br>
><br>
> While I haven't really had time, and I didn't even write the original patch,<br>
> I've already described exactly how to fix this by disabling the warning with<br>
> a version check in CMake... It should be really straightforward to<br>
> implement. I guess if for some reason none have done it I will, but I don't<br>
> even have a version of Clang to test it with, so I'm somewhat inclined for<br>
> someone who does to write this patch...<br>
><br>
Patch attached, to disable the warning when it gives false positives<br>
like that. OK to commit?<br>
</div></div></blockquote></div><br></div>