Closed Bug 1316230 Opened 7 years ago Closed 7 years ago

Incorrect LIR type names in Ion logs


(Core :: JavaScript Engine: JIT, defect, P5)




Tracking Status
firefox52 --- fixed


(Reporter: terpri, Assigned: terpri)



(1 file)

The definition of typeChars in js/src/jit/LIR.cpp is out of sync with the LDefinition::Type enum, resulting in incorrect Ion log output.
This is one possible fix; I replaced typeChars with a function, and gcc/clang should emit warnings (from -Wswitch) if someone forgets to update LIR.cpp.
Comment on attachment 8808910 [details] [diff] [review]
Update LIR type names for debugging output

Review of attachment 8808910 [details] [diff] [review]:

Looks good to me, thank you for the patch! I see that you are new to bugzilla but you might know the release process thanks to other people from Igalia :)

Just in case, to summarize:
- I've approved the patch (r+ for review+). Next time, you can take a look at who's touched the code you've modified with `hg blame`, and ask them for review by setting the flag to `r?` in bugzilla.
- Now somebody needs to land the patch. Ideally it should through our CI; we have testing CI called "try"; I'll push to try for you today. You can request commit access level 1 to push to try by yourselves, according to
- Then when we see that all the tests are passing, somebody can push to the integration CI (mozilla-inbound). By adding the `checkin-needed` keyword to the list of keywords of this bug, a sheriff would push it on your behalf, if there's a try push link and all the tests are passing. Otherwise, somebody with commit access level 3 needs to push this.

I've added `r=bbouvier` to the commit message, so it shows that somebody approved this change.

Here's the try link:
Attachment #8808910 - Flags: review+
Looks mostly green (the errors are not related to your patch). Setting checkin-needed so that a sheriff lands it on your behalf. Thanks again for the patch!
Keywords: checkin-needed
Assignee: nobody → robin
Ever confirmed: true
Pushed by
Update LIR type names for debugging output. r=bbouvier
Keywords: checkin-needed
Priority: -- → P5
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.