Last Comment Bug 693712 - SpiderMonkey dtrace build breakage
: SpiderMonkey dtrace build breakage
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: mozilla10
Assigned To: Steve Fink [:sfink] [:s:]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-11 11:29 PDT by Steve Fink [:sfink] [:s:]
Modified: 2011-10-12 03:13 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Header juggling (1.11 KB, patch)
2011-10-11 11:29 PDT, Steve Fink [:sfink] [:s:]
no flags Details | Diff | Splinter Review
Header juggling (1.60 KB, patch)
2011-10-11 11:55 PDT, Steve Fink [:sfink] [:s:]
wmccloskey: review+
Details | Diff | Splinter Review
Fix other SM build problems (5.05 KB, patch)
2011-10-11 12:42 PDT, Steve Fink [:sfink] [:s:]
wmccloskey: review+
Details | Diff | Splinter Review

Description Steve Fink [:sfink] [:s:] 2011-10-11 11:29:54 PDT
Created attachment 566279 [details] [diff] [review]
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 (:billm) 2011-10-11 11:34:22 PDT
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 (:billm) 2011-10-11 11:39:46 PDT
...although it looks like we can remove jsprobes.h from INSTALLED_HEADERS!
Comment 3 Steve Fink [:sfink] [:s:] 2011-10-11 11:55:40 PDT
Created attachment 566285 [details] [diff] [review]
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:] 2011-10-11 12:42:29 PDT
Created attachment 566302 [details] [diff] [review]
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 (:billm) 2011-10-11 13:45:57 PDT
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:] 2011-10-11 14:53:16 PDT
(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:] 2011-10-11 15:32:51 PDT
(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

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