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

[David Blaikie]

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.

>
>>
>>
>> -eric
>>
>>>>
>>>>
>>>> Modified:
>>>>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
>>>>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h
>>>>
>>>> Modified:
>>>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc?rev=177132&r1=177131&r2=177132&view=diff
>>>>
>>>> ==============================================================================
>>>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
>>>> (original)
>>>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
>>>> Thu Mar 14 19:20:17 2013
>>>> @@ -22,6 +22,8 @@ ThreadContextBase::ThreadContextBase(u32
>>>>    name[0] = '\0';
>>>>  }
>>>>
>>>> +ThreadContextBase::~ThreadContextBase() {}
>>>> +
>>>>  void ThreadContextBase::SetName(const char *new_name) {
>>>>    name[0] = '\0';
>>>>    if (new_name) {
>>>>
>>>> Modified:
>>>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h?rev=177132&r1=177131&r2=177132&view=diff
>>>>
>>>> ==============================================================================
>>>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h
>>>> (original)
>>>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h
>>>> Thu Mar 14 19:20:17 2013
>>>> @@ -34,6 +34,7 @@ enum ThreadStatus {
>>>>  class ThreadContextBase {
>>>>   public:
>>>>    explicit ThreadContextBase(u32 tid);
>>>> +  virtual ~ThreadContextBase();
>>>>
>>>>    const u32 tid;  // Thread ID. Main thread should have tid = 0.
>>>>    u64 unique_id;  // Unique thread ID.
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>>>
>>>
>>> --
>>> Alexey Samsonov, MSK
>>
>>
>
>
>
> --
> Alexey Samsonov, MSK
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>