All users were logged out of Bugzilla on October 13th, 2018

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

RESOLVED FIXED

Status

()

RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: glandium, Assigned: brendan)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
Adding #include "jscntxt.h" at the beginning of jsscope.h should be enough.
(Assignee)

Comment 1

9 years ago
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
(Reporter)

Comment 2

9 years ago
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)
(Assignee)

Comment 3

9 years ago
Created attachment 408215 [details] [diff] [review]
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
Assignee: general → brendan
Status: NEW → ASSIGNED
Attachment #408215 - Flags: review?(jorendorff)

Comment 4

9 years ago
Comment on attachment 408215 [details] [diff] [review]
patch

Stealing.
Attachment #408215 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 5

9 years ago
http://hg.mozilla.org/tracemonkey/rev/d0602cc15d62

/be
Whiteboard: fixed-in-tracemonkey

Comment 6

9 years ago
http://hg.mozilla.org/mozilla-central/rev/d0602cc15d62
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.