Remove flow sensitive alias analysis option

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: mgaudet, Assigned: Himanshuteli, Mentored)

Tracking

({good-first-bug})

Trunk
mozilla62
Points:
---

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

In bug 1455280 flow sensitive alias analysis was removed, but the option was left behind to simplify fuzzing and bisection. Nevertheless, at some point the option should go away.
(Reporter)

Updated

a year ago
Priority: -- → P3
(Assignee)

Comment 1

a year ago
I would like to work on this can you guide me
Hey, I'd love to guide you. 

First step: make sure you get a copy of the sources, and are able to build at least spidermonkey [1]. In my experience, using bootstrap.py is the easiest way to get ready to go [2].  

So the option used to be 'ion-aa'. A very useful tool for you is https://searchfox.org/, which will help you find the references to that option. If I recall correctly there are two of them. You'll just need to prepare a patch to excise them. 

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Build_Documentation
[2]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build/Linux_and_MacOS_build_preparation
(Assignee)

Comment 3

a year ago
I have successfully build firefox and compiled it.
If I'm correct, we need to remove the lines: 
if (op.getStringOption("ion-aa")) {}
|| !op.addStringOption('\0', "ion-aa", "flow-sensitive/flow-insensitive",
                               "Specify wheter or not to use flow sensitive Alias Analysis"
                               "(default: flow-insensitive)")
located in js/src/shell/js.cpp

Let's wait for more info.
Those look about right to me. So now, you'll do the removal, ensure things still build, then upload a patch to this bug.
(Assignee)

Comment 5

a year ago
Posted patch js.cpp (obsolete) — Splinter Review
Removed flow sensitive alias analysis option

Comment 6

a year ago
(In reply to Himanshuteli from comment #5)
> Created attachment 8975864 [details] [diff] [review]
> js.cpp
> 
> Removed flow sensitive alias analysis option

Please join #introduction on irc.mozilla.org, then I will help you create a patch.
(Assignee)

Comment 8

a year ago
Removed flow sensitive alias analysis option. I hope this patch is correct.
Attachment #8975864 - Attachment is obsolete: true
Assignee: nobody → Himanshuteli
Comment on attachment 8979027 [details] [diff] [review]
1460883-Remove-flow-sensitive-alias-analysis-option

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

Thanks for doing this! :)
Attachment #8979027 - Flags: review+
If you are happy with the patch, add "checkin-needed" to the |Keywords| list (which you can edit by clicking Edit Bug near the top of page). A Sheriff will add my name to the commit message and checkin the patch for us.
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 11

a year ago
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5184707be08
Remove flow sensitive alias analysis option r=tcampbell
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f5184707be08
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
\o/ Congrats on landing your first SpiderMonkey patch!
You need to log in before you can comment on or make changes to this bug.