1013065 - Bug 65664's NSCAP_DONT_PROVIDE_NONCONST_OPEQ autoconf test fails to compile with -Werror=uninitialized, yielding incorrect result and further compilation errors

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: remove-NSCAP_DONT_PROVIDE_NONCONST_OPEQ.patch

In bug 1001975, I'm trying to remove configure.in's -Wno-error=uninitialized (bug 716541) so clang and gcc will treat -Wuinitialized as warnings-as-errors in FAIL_ON_WARNINGS directories. clang currently reports no -Wuninitialized warnings and gcc reports only one (bug 1007708), so I may advocate for forcing -Werror=uninitialized across all mozilla-central, not just FAIL_ON_WARNINGS directories.

Unfortunately, when I enable -Werror=uninitialized (in configure.in or my mozconfig) instead of just -Wuninitialized, clang and gcc report hundreds of compilation errors about ambiguous comparisons of nsCOMPtrs and raw pointers:

xpcom/ds/nsObserverList.h:44:54: error: use of overloaded operator '==' is ambiguous (with operand types 'const nsCOMPtr<nsISupports>' and 'nsISupports *')
xpcom/ds/nsSupportsArray.cpp:268:27: error: use of overloaded operator '!=' is ambiguous (with operand types 'nsISupports *' and 'nsCOMPtr<nsISupports>')
xpcom/io/nsDirectoryService.cpp:512:14: error: use of overloaded operator '==' is ambiguous (with operand types 'nsCOMPtr<nsIAtom>' and 'nsIAtom *')
xpcom/threads/TimerThread.h:53:20: error: use of overloaded operator '==' is ambiguous (with operand types 'const nsCOMPtr<nsIThread>' and 'nsIThread *')
xpcom/threads/nsThread.cpp:886:42: error: use of overloaded operator '!=' is ambiguous (with operand types 'nsRefPtr<nsThread::nsNestedEventTarget>' and 'nsIEventTarget *')
../../dist/include/nsTArray.h:546:14: error: use of overloaded operator '==' is ambiguous (with operand types 'const nsCOMPtr<nsIThreadObserver>' and 'nsIThreadObserver *const')
...

Reverting your NSCAP_DONT_PROVIDE_NONCONST_OPEQ workaround from bug 65664 makes these errors go away. I have no idea why -Werror=uninitialized causes these seemingly unrelated errors when -Wuninitialized does not. I would assume this is a gcc bug except clang and gcc both report the same errors and on multiple platforms.

* -Wuninitialized is green:
https://tbpl.mozilla.org/?tree=Try&rev=76c4d06c12fa

* -Werror=uninitialized is red:
https://tbpl.mozilla.org/?tree=Try&rev=66398394f261

* -Werror=uninitialized without NSCAP_DONT_PROVIDE_NONCONST_OPEQ is green:
https://tbpl.mozilla.org/?tree=Try&rev=315356b10719

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8425244 [details] [diff] [review]
remove-NSCAP_DONT_PROVIDE_NONCONST_OPEQ.patch

I'm not sure whether autoconf tests are run with our default compilation flags, but it seems that the most likely theory for the problem here is that they are, so that flipping on -Werror=uninitialized is triggering a bogus failure of this autoconf test by causing a compilation error due to:

error: ‘bar’ is used uninitialized in this function [-Werror=uninitialized]

which was previously:

configure:25700:37: warning: 'bar' is used uninitialized in this function [-Wuninitialized]

(You should find this in config.log in the objdir.)


An easy way to test that would be to see if the problem still occurs if you add:
    Pointer() : myPtr(0) {}
to the definition of clas Pointer, and then also initialize bar to 0.

If that makes the problem go away, then we have an understanding of why it happened.

And I'm fine with removing the autoconf test itself, since it's probably no longer needed, so r=dbaron, as long as you fix the bug number in the commit message.


It might also be worth going through the autoconf output in config.log and looking for other compiler warnings in autoconf tests.

Comment 2

[Chris Peterson [:cpeterson]]

(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #1)
> Comment on attachment 8425244 [details] [diff] [review]
> remove-NSCAP_DONT_PROVIDE_NONCONST_OPEQ.patch
> 
> I'm not sure whether autoconf tests are run with our default compilation
> flags, but it seems that the most likely theory for the problem here is that
> they are, so that flipping on -Werror=uninitialized is triggering a bogus
> failure of this autoconf test by causing a compilation error due to:
> 
> error: ‘bar’ is used uninitialized in this function [-Werror=uninitialized]
> 
> which was previously:
> 
> configure:25700:37: warning: 'bar' is used uninitialized in this function
> [-Wuninitialized]

This "'bar' is used uninitialized" warning-as-error caused the autoconf test to return an unexpected result, which caused the original compilation errors.


> And I'm fine with removing the autoconf test itself, since it's probably no
> longer needed, so r=dbaron, as long as you fix the bug number in the commit
> message.

I could just fix the configure.in warning, but I'd prefer to remove unnecessary workaround because it complicates some C++ code. :)


> It might also be worth going through the autoconf output in config.log and
> looking for other compiler warnings in autoconf tests.

config.log reports other warnings, but they are in code generated for autoconf's implicit checks, not code from our configure.in. Fortunately, I don't think any of the warnings will cause configure problems.

Comment 3

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/66db03e176d4

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/66db03e176d4