853178 - Remove all SRC_PCDELTA source notes except on JSOP_CASE opcodes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

and rename SRC_PCDELTA to SRC_NEXTCASE.

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: v1

Comment 2

[Nicholas Nethercote [inactive]]

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

Review of attachment 728108 [details] [diff] [review]:
-----------------------------------------------------------------

Saving source code has really led to a never-ending stream of goodness, hasn't it?

This is great, except I can't see in this patch where SRC_NEXTCASE is used!  (Except for the case in shell/js.cpp, but that doesn't count.)  But looking at the codebase, I do see this line in IonBuilder.cpp, which you haven't changed:

    JS_ASSERT(caseSn && SN_TYPE(caseSn) == SRC_PCDELTA);

Are you building with --disable-ion?  And since the only place it's used is an assertion do we really need it?  I say just get rid of it completely.  So, f+ for now...

Comment 3

[Jason Orendorff [:jorendorff]]

(In reply to Nicholas Nethercote [:njn] from comment #2)
> at the codebase, I do see this line in IonBuilder.cpp, which you haven't
> changed:
> 
>     JS_ASSERT(caseSn && SN_TYPE(caseSn) == SRC_PCDELTA);

D'oh. This should've been changed too, of course; I think I accidentally qrecorded it into another patch somewhere.

> [...] And since the only place it's used is
> an assertion do we really need it?  I say just get rid of it completely. 

I think we should keep it for now. Ion uses the data in the source note, and every source note has a type.

Ideally we eventually stop using bytecode and then it'll go away. ;)

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: v2

Comment 5

[Nicholas Nethercote [inactive]]

Comment on attachment 729051 [details] [diff] [review]
v2

Review of attachment 729051 [details] [diff] [review]:
-----------------------------------------------------------------

Oh, I missed the post-assertion use in IonBuilder.cpp.  Looks good, thanks.

Comment 6

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bfc2073bcf31

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bfc2073bcf31