513119 - Change JSScript macros to methods

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: v1

I didn't turn JS_GET_SCRIPT_ATOM into a method. That's because it's not like the other ones. For one thing it captures cx from the lexical environment where it's called. Another issue is that bug 513119 is in that code, so I might be refactoring it in the near future anyway.

Comment 2

[Jeff Walden [:Waldo]]

Comment on attachment 397152 [details] [diff] [review]
v1

>diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp

>-    memcpy(SCRIPT_NOTES(wscript), snbase, nsrcnotes);
>+    memcpy(wscript->notes(), snbase, nsrcnotes);

nsrcnotes * sizeof(jssrcnote)?


>diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp

> /*
>  * JSScript data structures memory alignment:
>  *
>  * JSScript
>  * JSObjectArray    script objects' descriptor if JSScript.objectsOffset != 0,
>- *                    use JS_SCRIPT_OBJECTS(script) macro to access it.
>+ *                    use script->objects() macro to access it.
>  * JSObjectArray    script regexps' descriptor if JSScript.regexpsOffset != 0,
>- *                    use JS_SCRIPT_REGEXPS(script) macro to access it.
>+ *                    use script->regexps() macro to access it.
>  * JSTryNoteArray   script try notes' descriptor if JSScript.tryNotesOffset
>- *                    != 0, use JS_SCRIPT_TRYNOTES(script) macro to access it.
>+ *                    != 0, use script->trynotes() macro to access it.

I have a feeling these aren't macros any more, Toto.


>-JSBool
>-js_IsInsideTryWithFinally(JSScript *script, jsbytecode *pc)

Not to disagree, but why is this being removed?  Is it just dead code that somehow never caused compiler warnings?

Comment 3

[Brendan Eich [:brendan]]

Attachment: followup to make jsscriptinlines.h self-contained, avoid jsfun.h assumption in jsscript.h

Comment 4

[Jason Orendorff [:jorendorff]]

(In reply to comment #2)
> >-    memcpy(SCRIPT_NOTES(wscript), snbase, nsrcnotes);
> >+    memcpy(wscript->notes(), snbase, nsrcnotes);
> 
> nsrcnotes * sizeof(jssrcnote)?

Yes, for great justice.

> >-JSBool
> >-js_IsInsideTryWithFinally(JSScript *script, jsbytecode *pc)
> 
> Not to disagree, but why is this being removed?  Is it just dead code that
> somehow never caused compiler warnings?

Yes.

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

With followups:

changeset:   32211:390977f2943c
user:        Brendan Eich <[e-mail]>
date:        Thu Sep 10 10:54:33 2009 -0700
summary:     Fixes based on jorendorff's comments in bug 515273.

changeset:   32210:2e775a15b8c0
user:        Brendan Eich <[e-mail]>
date:        Thu Sep 10 10:23:52 2009 -0700
summary:     Fix js*inlines.h harder (515628, r=jorendorff).

changeset:   32209:d785fb2397f8
user:        Jason Orendorff <[e-mail]>
date:        Thu Sep 10 11:50:43 2009 -0500
summary:     Fix release-only linker error introduced by changeset 08ab547f6808 (having to do with jsscriptinlines.h). r=red.

changeset:   32208:08ab547f6808
user:        Brendan Eich <[e-mail]>
date:        Thu Sep 10 07:22:20 2009 -0700
summary:     Followup for 513119, r=jorendorff.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

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

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/08ab547f6808

Comment 7

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/1a3490a05458