[compiler-rt] r177132 - Fix a virtual destructor warning.

[Reid Kleckner]

On Tue, Mar 19, 2013 at 3:49 PM, Alexey Samsonov <samsonov at google.com>wrote:

> On Tue, Mar 19, 2013 at 8:12 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Tue, Mar 19, 2013 at 3:11 AM, Alexey Samsonov <samsonov at google.com>
>> wrote:
>> >
>> > On Fri, Mar 15, 2013 at 11:07 AM, Eric Christopher <echristo at gmail.com>
>> > wrote:
>> >>
>> >>
>> >>
>> >>
>> >> On Thu, Mar 14, 2013 at 11:21 PM, Alexey Samsonov <samsonov at google.com
>> >
>> >> wrote:
>> >>>
>> >>>
>> >>> On Fri, Mar 15, 2013 at 4:20 AM, Eric Christopher <echristo at gmail.com
>> >
>> >>> wrote:
>> >>>>
>> >>>> Author: echristo
>> >>>> Date: Thu Mar 14 19:20:17 2013
>> >>>> New Revision: 177132
>> >>>>
>> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=177132&view=rev
>> >>>> Log:
>> >>>> Fix a virtual destructor warning.
>> >>>>
>> >>>> Patch by Manuel Klimek!
>> >>>
>> >>>
>> >>> Thanks! Should we make this warning fire in CMake build +
>> >>> LLVM_ENABLE_WERROR?
>> >>>
>> >>
>> >>
>> >> Mmm. Probably.
>> >
>> >
>> > Done in r177385.
>>
>> Sorry I didn't reply earlier - but just to check, do we want this?
>>
>> Clang has a warning about virtual destruction as well that might be
>> sufficiently accurate without having to provide virtual dtors for
>> types that are used, but not owned, virtually. Hmm, seems that's off
>> by default too.
>>
>> So, alternate proposal:
>>
>> disable GCC's -Wnon-virtual-dtor
>> enable Clang's -Wdelete-non-virtual-dtor
>>
>> It catches things like:
>>
>> struct foo {
>>   virtual void bar();
>> };
>>
>> struct derived: foo { };
>>
>> int main() {
>>   foo *f = new derived;
>>   delete f;
>> }
>>
>> (without incorrectly warning on: void func(foo& f) { f.bar(); } int
>> main() { derived d; func(d); })
>>
>> Which could still be a false positive if such a pointer doesn't
>> actually point to various derived objects at runtime, but is probably
>> still a better signal than -Wnon-virtual-dtor.
>>
>
> FTR, -Wnon-virtual-dtor didn't show any warnings on LLVM/Clang code, and
> 3 (seemingly not true positive) warnings on lld code (which I silenced).
>
> Just to make sure we're on the same page - do you want
> to add "-Wno-non-virtual-dtor -Wdelete-non-virtual-dtor" to compile flags?
>
> This sounds fine to me, and it may indeed help with false positives, but
> my motivation for the change was patch by Manuel - it seems that some
> people
> actually compile LLVM codebase with -Wnon-virtual-dtor and expect it
> to compile w/o warnings.
>

I just caught one of these warnings in
http://llvm-reviews.chandlerc.com/D1170 before committing.  I have a class
(StringSaver) that I never intend to heap allocate or delete from a base
pointer, but -Wnon-virtual-dtor fires.

Maybe we should consider changing -Wnon-virtual-dtor to
"-Wdelete-non-virtual-dtor -Wno-non-virtual-dtor".  Would that break anyone?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130718/f07d9819/attachment.html>