Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#2462 closed enhancement (fixed)

[PATCH] SpiderMonkey ESR31 upgrade

Reported by: Yves Owned by: Yves
Priority: Should Have Milestone: Alpha 18
Component: Core engine Keywords: spidermonkey javascript
Cc: Patch:

Description (last modified by Stan)

Now that the ESR24 upgrade is done (#1886), I've started working on the ESR31 upgrade. You can test the current WIP version on my github repository (ESR31 branch). Read the readme there for additional information.

Depends on

#2669

When updating, don't forget to also fix #2684 and #1374 and #2973.

Change History (47)

comment:1 by Niek, 10 years ago

I thought SpiderMonkey 3.1 will be released in July 2014?

"The next release will be SpiderMonkey 31, around July 2014." https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey

comment:2 by Erik Johansson, 10 years ago

The better to start on the upgrade now rather than wait until after it's been released ;)

comment:3 by Niek, 10 years ago

But in that case you take the risk that you need to change drastically if the guys @Mozilla do so.

comment:4 by Yves, 10 years ago

There are two big advantages when starting with the upgrade now.

  1. If we find problems that need to be fixed in SpiderMonkey, we can still get these changes in before the next release. The Firefox (and SpiderMonkey) development starts in the mozilla central repository where big features get integrated. After that it moves to the "Aurora" stage where usually only important bugfixes get applied. This transition will be around the 28th of April, so it's good to start now.
  2. When updating around once a week, it's easier to figure out the reason if a change breaks something. You can check last week's commit logs or ask people what happened last week. That's harder if you have to search in a period of multiple months.

Sometimes there are bugs and it can take a while to figure out it's not a problem in our code, but the advantages outweight this problem.

comment:5 by Yves, 10 years ago

Description: modified (diff)

comment:6 by Yves, 10 years ago

C++11 support is required to use SpiderMonkey 31. I have checked today if patching the headers would be an alternative to avoid that, but it would result in too deep changes. I've replaced all the static_asserts, but then didn't investigate any further when I figured out that the headers also use move constructors. I've created ticket #2669 for the C++11 migration.

comment:7 by Yves, 10 years ago

In 15516:

Changes our JSNative functions to use JS::CallReceiver/JS::CallArgs.

This is the new way for working with arguments in JSNative functions. JS_THIS_VALUE, JS_ARGV, JS_SET_RVAL and direct access to vp or argc are deprecated and will probably be removed in future versions of SpiderMonkey.
CallArgs also takes care of proper rooting and you can get the values as Handles or MutableHandles. The interface changes a little bit for ESR 31, but commiting this now still makes it easier and the changes shout be straigtforward (search and replace more or less).

Refs #2462
Refs #2415

comment:8 by Yves, 10 years ago

In 15517:

Changes FromJSVal to take a JS::HandleValue instead of JS::Value.

JS::HandleValue is basically a wrapper around a JS::Value that is safe for exact stack rooting and moving GC.
I've tried to keep this changeset rather small and isolated and therefore create additional JS::Rooted<T> values at some places where the function should eventually directly take a JS::Handle<T>.
The functions "CallFunction" and "CallFunctionVoid" put their arguments inside a JS::AutoValueVector because this will be passed directly to "CallFunction_" with ESR31.

Refs #2462
Refs #2415

comment:9 by Yves, 10 years ago

In 15524:

Makes custom JS objects compatible with SpiderMonkey ESR31.

In v24 you called JS_InitClass and passed in a definition of JSNative functions. Later you could call JS_NewObject with this class and the object would get a prototype with the specified JSNative functions.
In ESR31 you now have to explicitly store the prototype object returned by JS_InitClass and pass it as prototype argument to JS_NewObject to achieve the same.
This change modifies our existing ScriptInterface implementation for custom object types a bit and uses it at places where the JSAPI was used directly before.

Refs #2462

comment:10 by Yves, 10 years ago

In 15534:

Changes ToJSVal to take JS::MutableHandleValue instead of JS::Value&.

JS::MutableHandleValue is similar to JS::HandleValue, but the whole JS::Value it points to can be replaced.
This change is needed for support of exact stack rooting and moving GC.
Contains a few other trivial API adjustments and style improvements too.

Refs #2462
Refs #2415

comment:11 by Yves, 10 years ago

In 15541:

Implements CallFunction with JS::MutableHandle<T> return type.

Changes the CallFunction implementation to use macros because otherwise we'd have to write twice as many functions manually.
Adapts GetSavedGameData to use the new function template. Additional callers will be changed in future commits.

Refs #2415
Refs #2462

comment:12 by Yves, 10 years ago

In 15542:

Adds support for passing JS::HandleValue to SetProperty and CallFunctionVoid without using ugly casing.

Also includes one little "demo-usecase", but additional code will be changed to use that in future commits.

Refs #2415
Refs #2462

comment:13 by sanderd17, 10 years ago

Description: modified (diff)

comment:14 by Yves, 10 years ago

In 15568:

Quite a lot of stack rooting related changes.

Changes GetProperty, SetProperty and HasProperty and a few other functions to take handles. The conversions to CScriptVal or CScriptValRooted at some places should be removed in the future. I've done that to avoid an even larger patch.

Refs #2415
Refs #2462

comment:15 by Yves, 10 years ago

In 15592:

More exact stack rooting (CallFunction object).

Changes CallFunction and CallFunctionVoid to use a HandleValue as object parameter. Also changes some JS serialization/deserialization functions to only support the JSAPI rooted types (drop support for CScriptVal and CScriptValRooted there). Some other functions got changed too because they were closely related.

Refs #2415
Refs #2462

comment:16 by Yves, 10 years ago

In 15597:

Exact stack rooting for structured cloning functions.

Refs #2415
Refs #2462

comment:17 by Yves, 10 years ago

In 15600:

Removes an unused codepath for AI deserialization and related ScriptInterface code.

It might actually be needed again in the future, but I think even then it would be easier to rewrite it than having to update the code in the meantime.

Refs #2462

comment:18 by Yves, 10 years ago

In 15601:

Exact rooting for CallConstructor.

Refs #2415
Refs #2462

comment:19 by Yves, 10 years ago

In 15603:

Exact stack rooting for JSON related ScriptInterface functions.

Refs #2415
Refs #2462

comment:20 by Yves, 10 years ago

In 15605:

Exact stack rooting for ScriptInterface::ToString.

I had to change a few other functions to take JS::MutableHandleValue because JS::Stringify takes a JS::MutableHandleValue as input parameter. That seems a bit strange because it should not change that value.
I assume it has historical reasons.

Refs #2415
Refs #2462

comment:21 by Yves, 10 years ago

In 15623:

Exact stack rooting for IGUIObject.

Refs #2415
Refs #2462

comment:22 by Yves, 10 years ago

Milestone: Alpha 17Alpha 18

comment:23 by Yves, 9 years ago

In 15944:

Exact stack rooting for CParamNode

Refs #2415
Refs #2462

comment:24 by Yves, 9 years ago

In 15946:

Replace CScriptValRooted with JS::Heap<T> and custom tracer for CNetClient

Refs #2462

comment:25 by leper, 9 years ago

Description: modified (diff)

#1374 can be considered fixed with SM29+.

comment:26 by Yves, 9 years ago

In 15961:

Split ScriptRuntime and ScriptInterface code to separate files.

The runtime is becoming more and more important in the JSAPI. As a result, we also have more functionality on the runtime level and having the whole ScriptRuntime class hidden in ScriptInterface.cpp doesn't make sense anymore. ScriptInterface.cpp also has become quite a large file and pulling out the runtime part makes it a bit smaller.

Refs #2462

comment:27 by Yves, 9 years ago

Keywords: review added
Summary: SpiderMonkey ESR31 upgrade[PATCH] SpiderMonkey ESR31 upgrade

The ESR31 branch is ready for review.

Open tests:

  • Test on the Mac

Notes:

  • I'm not using a new directory for the SpiderMonkey headers, this means they are part of the diff. You can make the diff more readable this way:
    git diff master --name-status | grep -v 'libraries/source/spidermonkey/include-win32' | xargs git diff master --
    

TODOs (before committing):

  • Add new Windows binaries compiled with the for .. of bugfix
  • Revert VS2010 compatibility patches (if agreed to drop support for VS versions <2013)

TODOs (C++11 related, before committing)

  • Agree definitely on dropping support for GCC < 4.6
  • Agree definitely on dropping support for Visual Studio < 2013

comment:28 by fabio, 9 years ago

Just a note that has_broken_pch can be removed from premake4.lua and the GCC check updated to fail with GCC < 4.6.

Also should with-c++11 GCC option forced enabled?

comment:29 by Yves, 9 years ago

In 16153:

Use future-proof lib path in build-osx-libs.sh. Patch by trompetin17.

There are symbolic links to the library binaries in the directory dist/lib. It's better to use these links rather than the location they point to for the copy command (they point to a different location in ESR31).

Refs #2462

comment:30 by Philip Taylor, 9 years ago

Some initial comments on the ESR31 branch:

  • When building (Ubuntu 14.04, gcc 4.8.2):
    In file included from ../../../source/third_party/tinygettext/src/dictionary_manager.cpp:20:0:
    ../../../source/third_party/tinygettext/include/tinygettext/dictionary_manager.hpp:88:59: warning: ‘auto_ptr’ is deprecated (declared at /usr/include/c++/4.8/backward/auto_ptr.h:87) [-Wdeprecated-declarations]
       void set_filesystem(std::auto_ptr<FileSystem> filesystem);
                                                               ^
    ../../../source/third_party/tinygettext/src/dictionary_manager.cpp:237:72: warning: ‘auto_ptr’ is deprecated (declared at /usr/include/c++/4.8/backward/auto_ptr.h:87) [-Wdeprecated-declarations]
     DictionaryManager::set_filesystem(std::auto_ptr<FileSystem> filesystem_)
                                                                            ^
    
    I guess we either want to fix our copy of tinygettext, or disable the warnings.
  • source/scriptinterface/third_party/ObjectToIDMap.h: The class is ObjectIdCache, so shouldn't this be ObjectIdCache.h?
  • LICENSE.txt: Added trailing whitespace. Also, quite a few other changes to source files are adding trailing whitespace. (Not hugely important, but it makes "git diff" unhappy.)
  • README.txt: Need to be careful not to apply that change to SVN.
  • build/premake/premake4.lua: Since the use of --with-c++11 was removed, the corresponding newoption line should be removed too.
  • build/workspaces/clean-workspaces.sh: Should continue to delete mozjs24 (at least for a few months), else people who update a dirty workspace will have mozjs24 hanging around forever.
  • libraries/osx/build-osx-libs.sh: Why change to $CPPFLAGS? That doesn't seem to be defined, and NSPR seems to include some C++ code so I'd guess it ought to still have CXXFLAGS set.
  • libraries/source/spidermonkey/: Might be nice if something like README.txt explained where all the .diff files come from and why they're needed (particularly so that people can tell when they're not needed any more and can be deleted).

in reply to:  30 comment:31 by Yves, 9 years ago

Replying to Philip:

Some initial comments on the ESR31 branch:

  • When building (Ubuntu 14.04, gcc 4.8.2):

...

I guess we either want to fix our copy of tinygettext, or disable the warnings.

I've seen those and I agree that we have to fix/disable them, but they are related to C++11 and not SpiderMonkey.

  • source/scriptinterface/third_party/ObjectToIDMap.h: The class is ObjectIdCache, so shouldn't this be ObjectIdCache.h?

They renamed the class to ObjectToIDMap in later versions of SpiderMonkey. I've first used the newer version and then copied the code from an older version. I think I'll leave it this way and it will match after the next upgrade.

  • LICENSE.txt: Added trailing whitespace. Also, quite a few other changes to source files are adding trailing whitespace. (Not hugely important, but it makes "git diff" unhappy.)

I've fixed most of these now.

  • README.txt: Need to be careful not to apply that change to SVN.

I've reverted the changes in README.txt just to be sure.

  • build/premake/premake4.lua: Since the use of --with-c++11 was removed, the corresponding newoption line should be removed too.

That was only a quick change to enable c++11 for this branch. I'm going to make a separate commit that fixes some clang issues, removes generation of unsupported workspaces and removes some workarounds for GCC < 4.2.

  • build/workspaces/clean-workspaces.sh: Should continue to delete mozjs24 (at least for a few months), else people who update a dirty workspace will have mozjs24 hanging around forever.

Done

  • libraries/osx/build-osx-libs.sh: Why change to $CPPFLAGS? That doesn't seem to be defined, and NSPR seems to include some C++ code so I'd guess it ought to still have CXXFLAGS set.

That's an issue, indeed. I've reverted an older version and haven't noticed the code has been changed in the meantime (r15410). Fixed.

  • libraries/source/spidermonkey/: Might be nice if something like README.txt explained where all the .diff files come from and why they're needed (particularly so that people can tell when they're not needed any more and can be deleted).

There's some description in build.sh and some patches contain some information in the header comment. All of them shouldn't be needed anymore in the next version. Two are backports and the third is a mix of backports and one new fix (tracelogger flushing). I've added some more information to these comments.

Last edited 9 years ago by Yves (previous) (diff)

comment:32 by ben, 9 years ago

In 16157:

Fixes build error 'no viable conversion' on OS X with clang, libc++ and c++11, refs #2462.
Removes explicit use of _Unwrap(), prefer deference operator * instead

in reply to:  30 comment:33 by historic_bruno, 9 years ago

Replying to Philip:

Some initial comments on the ESR31 branch:

  • When building (Ubuntu 14.04, gcc 4.8.2):
    In file included from ../../../source/third_party/tinygettext/src/dictionary_manager.cpp:20:0:
    ../../../source/third_party/tinygettext/include/tinygettext/dictionary_manager.hpp:88:59: warning: ‘auto_ptr’ is deprecated (declared at /usr/include/c++/4.8/backward/auto_ptr.h:87) [-Wdeprecated-declarations]
       void set_filesystem(std::auto_ptr<FileSystem> filesystem);
                                                               ^
    ../../../source/third_party/tinygettext/src/dictionary_manager.cpp:237:72: warning: ‘auto_ptr’ is deprecated (declared at /usr/include/c++/4.8/backward/auto_ptr.h:87) [-Wdeprecated-declarations]
     DictionaryManager::set_filesystem(std::auto_ptr<FileSystem> filesystem_)
                                                                            ^
    
    I guess we either want to fix our copy of tinygettext, or disable the warnings.

#2522 is relevant and should fix those warnings.

comment:34 by Yves, 9 years ago

I've successfully tested on Ubuntu 12.04 with GCC 4.6.

comment:35 by Stan, 9 years ago

Do I need VS2013 ?

in reply to:  35 comment:36 by Yves, 9 years ago

Replying to stanislas69:

Do I need VS2013 ?

Yes, from now on you need VS2013 to build on Windows. We have decided that it's not worth the additional efforts and trouble to support older versions with C++11. It's already a challenge to support three different platforms and compilers (not only with C++11, but it gets a bit harder now).

comment:37 by Stan, 9 years ago

Description: modified (diff)

comment:38 by Philip Taylor, 9 years ago

  • IGUIObject.cpp: Is setCompileAndGo appropriate here? I have vague memories that it was intended for functions which you compile and then execute immediately and exactly once (so it can apply optimisations at compile time that would be unsafe if some other script had a chance to run and modify the context's global state before running the newly compiled function), but GUI event handlers will be called many times.
  • IGUIObject::SetScriptHandler: The comment confused me, because there's only an indirect relationship between m_ScriptHandlers and the JSRuntime. Maybe something a bit like "m_ScriptHandlers is only rooted after SetGUI() has been called (which sets up the GC trace callbacks), so we can't safely store objects in it if the GUI hasn't been set yet." would be more explicit?
  • IGUIObject::TraceMember: Since we're in the brave new world of C++11, this should probably be
            for (auto& handler : m_ScriptHandlers)
                    JS_CallHeapObjectTracer(trc, &handler.second, "IGUIObject::m_ScriptHandlers");
    
  • IGUIObject.h: m_ScriptRuntime is never used.
  • NetServer.cpp:
    -       // Clear roots before deleting their context
    -       m_GameAttributes = CScriptValRooted();
    -       m_SavedCommands.clear();
    -
            SAFE_DELETE(m_ScriptInterface);
    
    Hmm, why is that not needed any more? It looks like JS::PersistentRooteds are stored in a linked list in their runtime, and the runtime is a shared_ptr owned by m_ScriptInterface so it will be destroyed here. Is there something that stops the m_GameAttributes/m_SavedCommands destructors from accessing that linked list in the now-deleted runtime?
  • NativeWrapperDecls.h: s/implicitely/implicitly/
  • NativeWrapperDecls.h: s/converson/conversion/
  • NativeWrapperDecls.h: A few CRLFs have crept into the file (around the "TODO: We allow ..." comment), while the rest of it is just LF. (At least on git on Linux.)
  • NativeWrapperDecls.h: A comment above the declaration of WrapperIfHandle might be nice, to explain roughly what it means.
  • ScriptInterface::LoadScript: The comment says CompileOptions doesn't copy the string; does JS_CompileUCFunction still copy it (so filenameStr only needs to stay alive until that function returns)? If so, I guess you could do JS_CompileUCFunction(..., JS::CompileOptions(m->m.cx).setFileAndLine(utf8_from_wstring(...).c_str(), ...).setCompileAndGo(true)); if you wanted to make the code a bit shorter (though I'm not sure whether that's a good idea or not).
  • ScriptInterface.h: s/explicitely/explicitly/. But I think we're not supporting VS2010 any more, so can the move operator/constructor be removed entirely?
  • ScriptRuntime::jshook_function: Hmm, that's removing the use of LocFlyweight, which I think will break the profiler. The profiler never copies strings, it just stores (and compares) const char* pointers, and assumes pointer equality is equivalent to string equality. But the new code is allocating a new string every time it's called, so the profiler will have pointers to deleted memory and also won't be able to match up two instances of the same string. LocFlyweight was added to make sure each string got allocated precisely once and remained valid forever (and also avoided any memory allocation cost for temporary strings in the cases where it could look up an already-allocated string), and I think it's still necessary.
  • ScriptRuntime.h: Comment of AddDeferredFinalizationObject has unmatched ).
  • ScriptTypes.h: Two copies of -Wignored-qualifiers.
  • ScriptTypes.h: Seems pointless to do a load of #pragma GCC diagnostic calls just before a #pragma GCC diagnostic pop which will undo all those changes. Should delete everything except the pop. (I guess that's a relic of older code that tried to be compatible with GCCs that didn't support pop.)
  • ScriptVal.h: s/explicitely/explicitly/, or just delete the move operator.
  • Simulation2.cpp: s/All 500 turns/Every 500 turns/
  • CCmpAIManager::Trace: for (auto& foo : bar) again.
  • CCmpCommandQueue::Serialize: Delete the declaration of tmpRoot.
  • ComponentManager.h: s/explicitely/explicitly/, or just delete all the VS2010 stuff.

in reply to:  38 comment:39 by Yves, 9 years ago

Replying to Philip:

  • IGUIObject.cpp: Is setCompileAndGo appropriate here? I have vague memories that it was intended for functions which you compile and then execute immediately and exactly once (so it can apply optimisations at compile time that would be unsafe if some other script had a chance to run and modify the context's global state before running the newly compiled function), but GUI event handlers will be called many times.

I did some research about CompileAndGo. My understanding is that it adds information specific to one global object to the compiled scirpts and it's not allowed to use these scripts with different global objects (but you may call them multiple times with the same global object). According to the description in bug 1095015, it's maybe even allowed to use different global objects, but that doesn't apply to us anyway. I'd say CompileAndGo is right there, but I'm not quite sure about what it really does exactly (when looking at bug 1095015 and related ones, I'd say not even Mozilla is quite sure about that and they are going to change it in the future).

  • IGUIObject::SetScriptHandler: The comment confused me, because there's only an indirect relationship between m_ScriptHandlers and the JSRuntime. Maybe something a bit like "m_ScriptHandlers is only rooted after SetGUI() has been called (which sets up the GC trace callbacks), so we can't safely store objects in it if the GUI hasn't been set yet." would be more explicit?

Indeed, that comment is better. Changed.

  • IGUIObject::TraceMember: Since we're in the brave new world of C++11, this should probably be
            for (auto& handler : m_ScriptHandlers)
                    JS_CallHeapObjectTracer(trc, &handler.second, "IGUIObject::m_ScriptHandlers");
    
  • IGUIObject.h: m_ScriptRuntime is never used.

Changed.

  • NetServer.cpp:
    -       // Clear roots before deleting their context
    -       m_GameAttributes = CScriptValRooted();
    -       m_SavedCommands.clear();
    -
            SAFE_DELETE(m_ScriptInterface);
    
    Hmm, why is that not needed any more? It looks like JS::PersistentRooteds are stored in a linked list in their runtime, and the runtime is a shared_ptr owned by m_ScriptInterface so it will be destroyed here. Is there something that stops the m_GameAttributes/m_SavedCommands destructors from accessing that linked list in the now-deleted runtime?

I think you are right, I didn't find anything that prevents this from happening. The problem is essentially still the same, but now with the runtime instead of the context and now it happens in SpiderMonkey code instead of our code. I've fixed this and added some comments to other places where something similar could happen.

  • NativeWrapperDecls.h: s/implicitely/implicitly/
  • NativeWrapperDecls.h: s/converson/conversion/

Fixed.

  • NativeWrapperDecls.h: A few CRLFs have crept into the file (around the "TODO: We allow ..." comment), while the rest of it is just LF. (At least on git on Linux.)

Should be fixed automatically when it's committed to SVN (svn:eol-style native).

  • NativeWrapperDecls.h: A comment above the declaration of WrapperIfHandle might be nice, to explain roughly what it means.

Done.

in reply to:  38 comment:40 by Yves, 9 years ago

Replying to Philip:

  • ScriptRuntime::jshook_function: Hmm, that's removing the use of LocFlyweight, which I think will break the profiler. [...]

Fixed here along with multiple other isses with the profiler. I'm too busy/lazy to split that up into multiple patches and figure out what can be applied to SVN first. I will probably commit them together with the SpiderMonkey upgrade.

in reply to:  38 comment:41 by Yves, 9 years ago

Replying to Philip:

  • ScriptRuntime.h: Comment of AddDeferredFinalizationObject has unmatched ).
  • ScriptTypes.h: Two copies of -Wignored-qualifiers.
  • ScriptTypes.h: Seems pointless to do a load of #pragma GCC diagnostic calls just before a #pragma GCC diagnostic pop which will undo all those changes. Should delete everything except the pop. (I guess that's a relic of older code that tried to be compatible with GCCs that didn't support pop.)
  • ScriptVal.h: s/explicitely/explicitly/, or just delete the move operator.
  • Simulation2.cpp: s/All 500 turns/Every 500 turns/
  • CCmpAIManager::Trace: for (auto& foo : bar) again.
  • CCmpCommandQueue::Serialize: Delete the declaration of tmpRoot.
  • ComponentManager.h: s/explicitely/explicitly/, or just delete all the VS2010 stuff.

Fixed. I've removed the VS2010 compatibility workarounds.

comment:42 by Yves, 9 years ago

Resolution: fixed
Status: newclosed

In 16214:

SpiderMonkey 31 upgrade

This upgrade also introduces exact stack rooting (see to the wiki: JSRootingGuide) and fixes problems with moving GC. This allows us to enable generational garbage collection (GGC).
Measurements a few months ago have shown a performance improvement of a non-visual replay of around 13.5%. This probably varies quite a bit, but it should be somewhere between 5-20%. Memory usage has also been improved. Check the forum thread for details.

Thanks to everyone from the team who helped with this directly or indirectly (review, finding and fixing issues, the required C++11 upgrade, the new autobuilder etc.)! Also thanks to the SpiderMonkey developers who helped on the #jsapi channel or elsewhere!

Fixes #2462, #2415, #2428, #2684, #1374
Refs #2973, #2669

comment:43 by Yves, 9 years ago

In 16215:

Fix build with Visual Studio

I've reverted this workaround before the SM31 commit because I thought it's only a problem with VS2010. Actually VS2013 still doesn't support C++11 well enough and still requires the workaround.

Refs #2669, #2462

comment:44 by Yves, 9 years ago

Keywords: review removed

comment:45 by historic_bruno, 9 years ago

The OS X Yosemite / clang build is very noisy due to warnings like these:

In file included from ../../../source/simulation2/components/CCmpPathfinder_Vertex.cpp:35:
In file included from ../../../source/simulation2/components/CCmpPathfinder_Common.h:30:
In file included from ../../../source/simulation2/system/Component.h:23:
In file included from ../../../source/simulation2/system/ComponentManager.h:23:
In file included from ../../../source/scriptinterface/ScriptInterface.h:25:
In file included from ../../../source/scriptinterface/ScriptTypes.h:68:
../../../libraries/source/spidermonkey/include-unix-release/jsapi.h:645:1: warning: 
      'JSFreeOp' defined as a struct here but previously declared as a class
      [-Wmismatched-tags]
struct JSFreeOp {
^
../../../libraries/source/spidermonkey/include-unix-release/js/Class.h:27:1: note: 
      did you mean struct here?
class JSFreeOp;
^~~~~
struct

In file included from ../../../source/gui/COList.cpp:18:
In file included from ../../../source/gui/COList.h:23:
In file included from ../../../source/gui/GUI.h:46:
In file included from ../../../source/gui/GUIutil.h:42:
In file included from ../../../source/gui/CGUI.h:43:
In file included from ../../../source/scriptinterface/ScriptInterface.h:25:
In file included from ../../../source/scriptinterface/ScriptTypes.h:68:
In file included from ../../../libraries/source/spidermonkey/include-unix-release/jsapi.h:24:
In file included from ../../../libraries/source/spidermonkey/include-unix-release/js/CallArgs.h:38:
../../../libraries/source/spidermonkey/include-unix-release/js/RootingAPI.h:1157:12: warning: 
      class 'PersistentRootedMarker' was previously declared as a struct
      [-Wmismatched-tags]
    friend class js::gc::PersistentRootedMarker<T>;
           ^
../../../source/scriptinterface/ScriptInterface.h:459:13: note: in instantiation
      of template class 'JS::PersistentRooted<JS::Value>' requested here
        handle.set(a);
                   ^

In file included from ../../../source/ps/SavedGame.cpp:20:
In file included from ../../../source/ps/SavedGame.h:21:
In file included from ../../../source/scriptinterface/ScriptInterface.h:25:
In file included from ../../../source/scriptinterface/ScriptTypes.h:68:
In file included from ../../../libraries/source/spidermonkey/include-unix-release/jsapi.h:21:
../../../libraries/source/spidermonkey/include-unix-release/jsalloc.h:22:1: warning: 
      class 'ContextFriendFields' was previously declared as a struct
      [-Wmismatched-tags]
class ContextFriendFields;
^
../../../libraries/source/spidermonkey/include-unix-release/jspubtd.h:326:8: note: 
      previous use is here
struct ContextFriendFields
       ^
../../../libraries/source/spidermonkey/include-unix-release/jsalloc.h:22:1: note: 
      did you mean struct here?
class ContextFriendFields;
^~~~~
struct

If we can't fix them properly, they should probably be disabled.

comment:46 by fabio, 9 years ago

I am not able to build with an external mozjs31 library. I specified --with-system-mozjs31 but after collada is built is starts building bundled spidermonkey. I always used the external spidermonkey with mozjs24.

comment:47 by Yves, 9 years ago

In 16235:

Fixes some missing replacements of mozjs24 with mozjs31

This has caused the bundled library to be built even if --with-system-mozjs31 is specified (but actually it has still correctly used the system library after building the bundled library).
Refs #2462

Note: See TracTickets for help on using tickets.