BrowserWorks / Waterfox-Classic

The Waterfox Classic repository, for legacy systems and customisation.
https://classic.waterfox.net
Mozilla Public License 2.0
175 stars 34 forks source link

SpiderMonkey backports part 2 (optional chaining) #127

Closed buttercookie42 closed 2 years ago

buttercookie42 commented 2 years ago

This part 2 of my backporting series, dealing with optional chaining.

Following Seamonkey's example, this one is relatively directly based on @buc-me's patches, and therefore also quite short.

I can't absolutely vouch for the correctness of the "Work around missing JSOP_GOTO functionality" (my SpiderMonkey knowledge doesn't go that far), but I've been using it for the past few weeks in my local build and didn't notice anything untoward happening. (Also Seamonkey 2.15.13b1 has included it, too)

Palemoon on the other hand has an alternative implementation that adds a new source note type for this, though at least right now I absolutely can't judge it for correctness, either. Maybe something to look into at some later point…

The one thing I have taken from Palemoon is the adaptation of autocomplete support in the console, because the original bug 1594009 would have required quite a few more changes in the devtools code. Somewhat miraculously, the corresponding xpcshell test actually even passes.

buc-me commented 2 years ago

buttercookie42 wrote:

I can't absolutely vouch for the correctness of the "Work around missing JSOP_GOTO functionality" (my SpiderMonkey knowledge doesn't go that far), but I've been using it for the past few weeks in my local build and didn't notice anything untoward happening. (Also Seamonkey 2.15.13b1 has included it, too)

Well, the enough description is in the header of that patch. It is kinda trivial change in the code logic. You can avoid this patch at all -- then js code containing optchains just remains "uncompiled", with less performance etc. (It seems PM left this uncompiled, btw).

Anyway, you can try to check the results with https://bakkot.github.io/test262-web-runner/ (but it probably requires service workers implemented/enabled to run).

buttercookie42 commented 2 years ago

I've read your commit message, and no objections there, the principle of that workaround looks absolutely sensible. It's just that while in the recent weeks I've now seen enough of the BytecodeEmitter code that it's no longer total gibberish to me, I'm still not supremely confident around that code area and how to translate from your commit message to the actual code changes (I'd probably have to sit down and trace out the generated byte code with a pen and paper, but to be honest I've been too lazy for that). But like I said, at least empirically it seems to work quite fine.

(It seems PM left this uncompiled, btw).

Not quite, they added a new source note type like I said, but weren't entirely sure about it, either ("XXX Instead of aborting early, breaking at this point works. However, I'm not sure if we still need to further process optional chains under IonBuilder.") – but they committed it anyway and haven't backed it out again, so it might actually work that way, too.