748237 - It's not possible to include a header using jshashtable from any dir that uses FAIL_ON_WARNINGS=1 due to android bustage in MoveRef

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Background: Android builds in c++11 mode.  So this code in MoveRef gets compiled:

    operator T&& ()  const { return static_cast<T&&>(*pointer); }

which leads to this lovely warning:

  ../../../dist/include/js/Utility.h:860: warning: returning reference to temporary

and in particular it happens due to this chain of template instantiations:

../../../dist/include/js/Utility.h: In member function 'js::MoveRef<T>::operator T&&() const [with T = JSObject*]':
../../../dist/include/js/HashTable.h:916:   instantiated from 'void js::HashMapEntry....
../../../dist/include/js/HashTable.h:84:   instantiated from 'void js::detail::HashTableEntry<T>::operator=(js::MoveRef<js::detail::HashTableEntry<T> >) 
../../../dist/include/js/HashTable.h:595:   instantiated from 'bool js::detail::HashTable<T, HashPolicy, AllocPolicy>::changeTableSize(int) 
../../../dist/include/js/HashTable.h:635:   instantiated from 'bool js::detail::HashTable<T, HashPolicy, AllocPolicy>::checkOverloaded() 
../../../dist/include/js/HashTable.h:263:   instantiated from 'js::detail::HashTable<T, HashPolicy, AllocPolicy>::Enum::~Enum()
/builds/slave/try-andrd-xul/build/js/xpconnect/wrappers/../src/XPCMaps.h:752:   instantiated from here

Sadly, XPCMaps.h is includes by xpcprivate.h, which is included by XPCQuickStubs.h, which is included by the new-binding Utils.h.  So trying to use new-bindings in any dir that has FAIL_ON_WARNINGS makes it fail on Android.

That said, the warning here sounds pretty bad.  Is it really meant to be happening?

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Only do the c++11 stuff for clang here, since otherwise we get scary warnings with gcc on android.

This compiles locally for me with clang and passes a try push on Android

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 618462 [details] [diff] [review]
Only do the c++11 stuff for clang here, since otherwise we get scary warnings with gcc on android.

Right on.

Comment 3

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/ecfe99b2c9cc

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ecfe99b2c9cc