Last Comment Bug 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
: It's not possible to include a header using jshashtable from any dir that use...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: 745897
  Show dependency treegraph
 
Reported: 2012-04-23 22:31 PDT by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2012-04-26 10:31 PDT (History)
4 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Only do the c++11 stuff for clang here, since otherwise we get scary warnings with gcc on android. (1.24 KB, patch)
2012-04-25 15:42 PDT, Boris Zbarsky [:bz] (still a bit busy)
luke: review+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2012-04-23 22:31:53 PDT
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 [:bz] (still a bit busy) 2012-04-25 15:42:11 PDT
Created attachment 618462 [details] [diff] [review]
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] 2012-04-25 15:53:11 PDT
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 [:bz] (still a bit busy) 2012-04-25 21:43:40 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/ecfe99b2c9cc
Comment 4 Ed Morley [:emorley] 2012-04-26 10:31:24 PDT
https://hg.mozilla.org/mozilla-central/rev/ecfe99b2c9cc

Note You need to log in before you can comment on or make changes to this bug.