Remove flow sensitive alias analysis option

RESOLVED FIXED in Firefox 62



a year ago
a year ago


(Reporter: mgaudet, Assigned: Himanshuteli, Mentored)




Firefox Tracking Flags

(firefox62 fixed)



(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.


a year ago
Priority: -- → P3

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 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, 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. 


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.

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, then I will help you create a patch.

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]

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.


a year ago
Keywords: checkin-needed

Comment 11

a year ago
Pushed by
Remove flow sensitive alias analysis option r=tcampbell
Keywords: checkin-needed
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.