693712 - SpiderMonkey dtrace build breakage

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

Attachment: Header juggling

jsprobes.h uses JSObject and JSScript when compiling with --enable-dtrace, so it needs to include the appropriate headers.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I think this is going to be a problem, since jsprobes.h is in INSTALLED_HEADERS and jsobj.h is not.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

...although it looks like we can remove jsprobes.h from INSTALLED_HEADERS!

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Header juggling

Oh, I put jsprobes.h in INSTALLED_HEADERS? Ugh. I guess that was before I knew better.

I'd actually like to create something someday that *would* go into INSTALLED_HEADERS, but jsprobes.h in its current form is not that.

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: Fix other SM build problems

Misusing this bug for further followup problems in SM builds.

This patch fixes 2 problems: (1) a warning where I initialize class members out of order, and (2) an error where I use mjit:: stuff when --disable-methodjit is on.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 566302 [details] [diff] [review]
Fix other SM build problems

Thanks!

By the way, it's outside the scope of this bug, but I noticed that jsprobes.h uses different capitalization than the rest of SM. I think normally it would be probes::DiscardExecutableRegion, rather than Probes::discardExecutableRegion (lowercase for namespaces and uppercase for top-level functions).

Comment 6

[Steve Fink [:sfink] [:s:]]

(In reply to Bill McCloskey (:billm) from comment #5)
> Comment on attachment 566302 [details] [diff] [review] [diff] [details] [review]
> Fix other SM build problems
> 
> By the way, it's outside the scope of this bug, but I noticed that
> jsprobes.h uses different capitalization than the rest of SM. I think
> normally it would be probes::DiscardExecutableRegion, rather than
> Probes::discardExecutableRegion (lowercase for namespaces and uppercase for
> top-level functions).

Yeah, you're right. I should fix that. Everything used to be in a js::Probes class, and I should've switched Probes::stuff to probes::Stuff at the same time.

Comment 7

[Steve Fink [:sfink] [:s:]]

(In reply to Steve Fink [:sfink] from comment #6)
> (In reply to Bill McCloskey (:billm) from comment #5)
> > Comment on attachment 566302 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > Fix other SM build problems
> > 
> > By the way, it's outside the scope of this bug, but I noticed that
> > jsprobes.h uses different capitalization than the rest of SM. I think
> > normally it would be probes::DiscardExecutableRegion, rather than
> > Probes::discardExecutableRegion (lowercase for namespaces and uppercase for
> > top-level functions).
> 
> Yeah, you're right. I should fix that. Everything used to be in a js::Probes
> class, and I should've switched Probes::stuff to probes::Stuff at the same
> time.

Filed bug 693838

Comment 8

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/c515dbded0c2
https://hg.mozilla.org/mozilla-central/rev/9ee34cc1d186