831633 - Hundreds of -Wdelete-non-virtual-dtor warnings

Status: RESOLVED FIXED

(Core :: General, defect, P4)

Description

[Chris Peterson [:cpeterson]]

Attachment: Wdelete-non-virtual-dtor-warnings.txt

Yesterday, I only saw a couple -Wdelete-non-virtual-dtor warnings when building Firefox for Android or Mac. Today, I see over 700:

content/base/src/EventSource.cpp:549:1 [-Wdelete-non-virtual-dtor] delete called on 'mozilla::dom::AsyncVerifyRedirectCallbackFwr' that has virtual functions but non-virtual destructor
...

I believe the regression is in this pushlog range:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=43391698ce5e&tochange=a4b246fb54c1

Comment 1

[Nicholas Nethercote [inactive]]

None of the patches in that range looks like an obvious culprit to me.  cpeterson, can you bisect?

Comment 2

[Chris Peterson [:cpeterson]]

glandium, these warnings are a regression from disabling MOZ_FINAL in bug 830315.

Can we fix bug 830315's warnings with something like -Wno-c++11-extensions? If clang supports `final` in pre-C++11 code, why not take advantage of it?

Comment 3

[Jeff Walden [:Waldo]]

Yeah, -Wno-c++0x-extensions and undoing the changes in bug 830315 is probably the best thing to do here.  That, or getting everything compiling as C++11, but that's kind of scope-creepy.  :-)  Still something we should do eventually.

Comment 4

[Chris Peterson [:cpeterson]]

Attachment: allow-clang-c++11.patch

waldo: this patch just backs out the change from bug 830315 that restricted clang's MOZ_FINAL and MOZ_DELETE to pre-C++11.

glandium: In which files did you the C++11 extension warnings in bug 830315? I see that configure.in and js/src/configure.in already add -Wno-c++0x-extensions to clang's CXXFLAGS:

https://hg.mozilla.org/mozilla-central/file/a207f33adc1a/configure.in#l1518

(I'm building on Mac OS X with Apple's clang 4.1.)

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 706278 [details] [diff] [review]
allow-clang-c++11.patch

Review of attachment 706278 [details] [diff] [review]:
-----------------------------------------------------------------

...hum, that's right, we do have that commandline flag.  I'd forgotten about that.  So, yeah, why were you seeing any warnings here, glandium?

Comment 6

[Mike Hommey [:glandium]]

Because the warning is:
  warning: deleted function definitions are a C++11 extension [-Wc++11-extensions]

Note it's -Wc++11-extensions, not -Wc++0x-extensions

Comment 7

[Mike Hommey [:glandium]]

Ah no, it's a different issue, it's because the warning flags are not given to HOST_CXX.

Comment 8

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/88fc571a9e9e

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Fixed by backout.
https://hg.mozilla.org/mozilla-central/rev/88fc571a9e9e