Wdelete-non-virtual-dtor doesn't work with std::unique_ptr

[nico]

Bugzilla Link	28460
Resolution	FIXED
Resolved on	Sep 02, 2017 12:47
Version	unspecified
OS	Linux
CC	@zetafunction,@DougGregor

Extended Description

This warns, like it should:

thakis@thakis:~/src/chrome/src$ cat test.cc
#include <memory>

struct S {
  virtual void f() {}
};

struct T : public S {
};

void f(S* s) {
  delete s;
}

thakis@thakis:~/src/chrome/src$ third_party/llvm-build/Release+Asserts/bin/clang -c test.cc -Wall -std=c++11 
test.cc:11:3: warning: delete called on non-final 'S' that has virtual functions but non-virtual destructor [-Wdelete-non-virtual-dtor]
  delete s;
  ^
1 warning generated.

But this doesn't:

thakis@thakis:~/src/chrome/src$ cat test.cc
#include <memory>

struct S {
  virtual void f() {}
};

struct T : public S {
};

void f() {
  std::unique_ptr<S> s;
}

thakis@thakis:~/src/chrome/src$ third_party/llvm-build/Release+Asserts/bin/clang -c test.cc -Wall -std=c++11 

It should.

[nico]

sdefresne, on the chromium bug tracker:

According to http://stackoverflow.com/a/26321378/5353, the issue comes from the fact that the delete happens in a standard header (where std::unique_ptr<> destructor is implemented), and can be restored with -Wsystem-headers, but this brings lots of noise."""

We should come up with some approach that ideally makes things Just Work for unique_ptr and -Wdelete-non-virtual-dtor. Failing that, maybe we could have something like -Wsystem-headers=delete-non-virtual-dtor (which would be nice for other things too, but it'd do the wrong thing by default for the bug at hand, since people need to opt in to it).

[zetafunction]

When I asked about this on IRC, someone mentioned that there's already a way to enable a diagnostic to fire, even if it's in a system header.

https://github.com/llvm-mirror/clang/blob/a0badfbffbee71c2c757d580fc852d2124dadc5a/include/clang/Basic/Diagnostic.td#L75

So I think maybe we just need to set this for true for NonVirtualDtor warnings?

[nico]

https://reviews.llvm.org/D37235

[nico]

r312167

[nico]

r312216 too