1284441 - Remove unnecessary cast from ExpressionDecompiler::decompilePC.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

https://dxr.mozilla.org/mozilla-central/rev/c9a70b64f2faa264296f0cc90d68a2ee2bac6ac5/js/src/jsopcode.cpp#1216
>         return decompilePCForStackOperand(pc, -int32_t(3)) &&

The cast there should be removed.

Comment 1

[archit garg[:archit]]

Hey
I'll work on this bug. Can you assign this to me?

Comment 2

[Tooru Fujisawa [:arai]]

Thanks!

Comment 3

[Jinank Jain[:jj]]

Attachment: a1.diff

Does this help in fixing?

Comment 4

[Tooru Fujisawa [:arai]]

Comment on attachment 8773516 [details] [diff] [review]
a1.diff

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

Thank you Jinank :D

Comment 5

[Tooru Fujisawa [:arai]]

Re-assigning to Jinank.

:archit, if you're looking for a bug to work on, feel free to ping me ;)

Comment 6

[Jinank Jain[:jj]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0f1dd47a941

https://hg.mozilla.org/try/rev/40b482c0c369

Comment 7

[Tooru Fujisawa [:arai]]

Sorry, I overlooked the commit message.
Can you set appropriate commit message that follows the rule in this document, and re-post it?
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities#Checkin_comment

After that, set "checkin-needed" keyword to this bug, or ni? me if you cannot.

Comment 8

[Jinank Jain[:jj]]

Attachment: a2.diff

Comment 9

[Tooru Fujisawa [:arai]]

Comment on attachment 8773779 [details] [diff] [review]
a2.diff

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

Please remove "[mq]: bug1284441" part from commit message.

Also "checkin+" means the patch have already checked in, so you don't need to touch the flag.

Comment 10

[Jinank Jain[:jj]]

Attachment: a2.diff

Comment 11

[Tooru Fujisawa [:arai]]

Comment on attachment 8773913 [details] [diff] [review]
a2.diff

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

Thanks, the commit message is now okay, however, somehow the patch file is corrupted.

> @@ -1213,7 +1213,7 @@ ExpressionDecompiler::decompilePC(jsbyte
>          return decompilePCForStackOperand(pc, -int32_t(GET_ARGC(pc) + 2)) &&
>                          write("(...)");
>                                 case JSOP_SPREADCALL:
>                                 -        return decompilePCForStackOperand(pc, -int32_t(3)) &&
>                                 +        return decompilePCForStackOperand(pc, -3) &&
>                                                 write("(...)");
>                                                        case JSOP_NEWARRAY:
>                                                                 return write("[]");)

So, please discard this patch, and import attachment 8773779 [details] [diff] [review] again, then modify the commit message with the following command:

hg qref -m 'Bug 1284441 - Remove unnecessary cast from ExpressionDecompiler::decompilePC. r=arai'

Comment 12

[Jinank Jain[:jj]]

Attachment: a2.diff

Comment 13

[Tooru Fujisawa [:arai]]

Comment on attachment 8773921 [details] [diff] [review]
a2.diff

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

Thanks, perfect!

Comment 14

[Jinank Jain[:jj]]

(In reply to Tooru Fujisawa [:arai] from comment #13)
> Comment on attachment 8773921 [details] [diff] [review]
> a2.diff
> 
> Review of attachment 8773921 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks, perfect!

Any other bugs that I could work on?

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/850f1a2a24cf
Remove unnecessary cast from ExpressionDecompiler::decompilePC. r=arai

Comment 16

[Ryan VanderMeulen [:RyanVM]]

1) This bug isn't invalid or you presumably wouldn't have made a patch for it.
2) Bugs get resolved automatically once the patch is merged to mozilla-central, so please don't manually close out the bug without a good reason for doing so.

Thanks for the patch!

Comment 17

[Tooru Fujisawa [:arai]]

(In reply to Jinank Jain from comment #14)
> Any other bugs that I could work on?

how about bug 1020571 ?

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/850f1a2a24cf

Comment 19

[Jinank Jain[:jj]]

(In reply to Tooru Fujisawa [:arai] from comment #17)
> (In reply to Jinank Jain from comment #14)
> > Any other bugs that I could work on?
> 
> how about bug 1020571 ?

This bugs seems to be fixed by ICU team then mozilla would just take that patch from there.