Fixed Function.prototype.toString as per the latest specification

[tuchida]

ref. #1188
Spec. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/toString#getting_source_text_of_a_function

(function() { /* comment */ if(1){}}).toString()

Expect

function() { /* comment */ if(1){}}

But

function () {
    if (1) {
    }
}

To fixed this, Decompiler has been removed.

[p-bakker]

Any idea if it covers all function .toString() variations mentioned in #1300?

[p-bakker]

@gbrail in #1188 (comment) you mentioned that you'd like to have backwards compatibility

Using GitHub's improved codesearch, I looked for Java files containing an import of the Decompiler and a call to the decompile function: https://github.com/search?q=org.mozilla.javascript.Decompiler+AND+decompile%28+language%3AJava&type=code&l=Java

This yields only a few results, most either of clones of Rhino or deprecated/abandoned projects (vjet, dltk) or projects that haven't had updates for years.

I'd say we bite the bullet and remove the Decompiler completely, even if this will go into 1.7.xx

[p-bakker]

Does it make sense to call this DecompilerFlag if there's no decompiling going on anymore?

[tuchida]

Do you have any good names?

[p-bakker]

@tuchida Maybe I'm overlooking something or you did it on purpose, but it looks to me that the Decompiler class hasn't been removed

[tuchida]

@p-bakker Thanks! I removed. ref. bff0cd1

Any idea if it covers all function .toString() variations mentioned in #1300?

Some minor modifications may be necessary to support all Function.prototype.toString. I would like to separate Pull Requests. ref. #1300 (comment)

[gbrail]

I'm fine with removing Decompiler -- we have a bunch of other big changes with this release and if we have to say, "instead of Decompiler, use toString()" then I think we will be OK.

[gbrail]

Other than that, I see that this PR changes a lot of legacy test suites because the output changed. I had a quick look and I'm prett sure that none of the output changes are significant, so changing those test suites is OK with me, especially because we enable a bunch of test262 suite tests.

Are you fairly confident that these changes are benign?

Finally, you have two big changes to IRFactory in the queue. git will probably handle them OK but there's no guarantee, especially since the other one has a bunch of places where you wrap things with braces and change the indent level. Do you have a preference as to which one gets merged first?

[tuchida]

Are you fairly confident that these changes are benign?

In terms of being on spec, Yes. I do not know the impact of breaking changes.

Do you have a preference as to which one gets merged first?

No, either is fine.

[gbrail]

Thanks for working on this -- I agree it's a good PR. Now that I merged your other one, this one needs conficlit resolution, so can you please do that and push a new commit? Thanks!

[tuchida]

@gbrail Thanks! I resolved the conflicts.

[gbrail]

Thanks for doing this!

[rbri]

@tuchida fantastic!!!! ❤️

Now i can remove my hack from the HtmlUnit fork of rhino. Thanks a lot.... hope to see more of you work gets merged soon