Re: [PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

2017-08-30 Thread Richard Smith via cfe-commits
On 28 August 2017 at 17:00, Nico Weber via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On Mon, Aug 28, 2017 at 7:40 PM, Richard Smith - zygoloid via Phabricator > via cfe-commits wrote: > >> rsmith added inline comments. >> >> >> >> Comment at: test/SemaCXX/destructor.cpp:

[PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

2017-08-30 Thread Nico Weber via Phabricator via cfe-commits
thakis closed this revision. thakis added a comment. Thanks, landed in r312167. dblaikie: I believe the test can't be much simpler -- the pragma is file-based and having the main TU be marked as system header would be fairly different from what happens in real life even if it worked. https://

Re: [PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

2017-08-28 Thread Nico Weber via cfe-commits
On Mon, Aug 28, 2017 at 7:40 PM, Richard Smith - zygoloid via Phabricator via cfe-commits wrote: > rsmith added inline comments. > > > > Comment at: test/SemaCXX/destructor.cpp:27 > +#define BE_THE_HEADER > +#include __FILE__ > + > > Do we guarantee that `__FILE_

[PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

2017-08-28 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: test/SemaCXX/destructor.cpp:27 +#define BE_THE_HEADER +#include __FILE__ + Do we guarantee that `__FILE__` names a path that can be used to include the current file? In other tests, we add `-include %s` to the `RUN:` lin

[PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

2017-08-28 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. (if you'd prefer to wait for Reid or Richard to take a look, that's OK too :) ) Looks good to me, though wouldn't mind if the tests were a bit simpler - if they can be made so.

[PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

2017-08-28 Thread Nico Weber via Phabricator via cfe-commits
thakis updated this revision to Diff 112981. thakis edited the summary of this revision. thakis added a comment. Don't warn in unevaluated contexts. Ready for a look now. https://reviews.llvm.org/D37235 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExprCXX.cpp test/SemaCX

Re: [PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

2017-08-28 Thread Nico Weber via cfe-commits
I had compiled a bunch of code with this, but right after I sent this out my local build of something failed with this: ../../buildtools/third_party/libc++/trunk/include/type_traits:2156:51: error: destructor called on non-final 'Ice::VariableDeclaration::RelocInitializer' that has virtual functio

[PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

2017-08-28 Thread Nico Weber via Phabricator via cfe-commits
thakis created this revision. Makes the warning useful again in a std::unique_ptr world, PR28460. https://reviews.llvm.org/D37235 Files: include/clang/Basic/DiagnosticSemaKinds.td test/SemaCXX/destructor.cpp Index: test/SemaCXX/destructor.cpp ==