524267 - js_LeaveTraceIfGlobalObject used in inline code in jsscope.h, but jscntxt.h is not included

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Mike Hommey [:glandium]]

Adding #include "jscntxt.h" at the beginning of jsscope.h should be enough.

Comment 1

[Brendan Eich [:brendan]]

Good point, although instead of overincluding for implementation dependencies (as opposed to interface dependencies, e.g. on typenames or array dimension manifest constants), we are moving inline implementations into js*inlines.h, so in this case we'd move JSScope::extend's body into jsscopeinlines.h and make any .cpp files that need extend explicitly include that .h file.

Are you including jsscope.h in some file that doesn't include jscntxt.h, or did you find this by inspection? Every includer in Mozilla builds happens to include jscntxt.h first, so I wondered if you had a reason to include jsscope.h in your embedding. No problem if it was a spurious over-include you fixed; I'm always on the lookout for embedding requirements not met by the public API.

/be

Comment 2

[Mike Hommey [:glandium]]

I caught that in perl javascript.pm cpan extension build logs, though the version of spidermonkey the build used was oldish and it wasn't JSScope::extend that was using js_LeaveTraceIfGlobalObject, but js_MakeScopeShapeUnique. I checked that this still applied on mozilla-central (though I guess I should have checked tracemonkey)

Comment 3

[Brendan Eich [:brendan]]

Attachment: patch

Or, we could just include jscntxt.h in jsscope.h :-P. Is this better? It seems like a win to segregate implementation from interface and avoid dependencies on the former where the latter suffices.

/be

Comment 4

[Andreas Gal :gal]

Comment on attachment 408215 [details] [diff] [review]
patch

Stealing.

Comment 5

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/d0602cc15d62

/be

Comment 6

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/d0602cc15d62