8258147: Modernize Nashorn code #5
Conversation
…rameter in NashornBottomLinker)
|
👋 Welcome back attila! A progress list of the required criteria for merging this PR into |
| @@ -829,7 +829,7 @@ public Object eval(final ScriptObject initialScope, final String string, | |||
|
|
|||
| Class<?> clazz; | |||
| try { | |||
| clazz = compile(source, new ThrowErrorManager(), strictFlag, true); | |||
| clazz = compile(source, new ThrowErrorManager(), strictFlag); | |||
There was a problem hiding this comment.
I vaguely isEval or not was used in compilation to compile differently. Or perhaps only for loading post compilation? (eval code as throw-away anon class?). But if the current code does not use it, then fine.
| @@ -82,7 +82,7 @@ public URLReader(final URL url, final Charset cs) { | |||
| } | |||
|
|
|||
| @Override | |||
| public int read(final char cbuf[], final int off, final int len) throws IOException { | |||
| public int read(final char[] cbuf, final int off, final int len) throws IOException { | |||
There was a problem hiding this comment.
Oops. Never knew we had those in the code :)
Did you run any tool to find these or just found while code inspection/reading?
There was a problem hiding this comment.
IntelliJ IDEA warns about them. Most of this code changes were driven by IDEA highlighting things.
|
@szegedi This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
|
/integrate |
|
@szegedi Since your change was applied there have been 3 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit e782364. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Most of Nashorn code was written to be compatible with Java 7 and never updated. It is high time to introduce diamonds, lambdas, modern collection operations (
List.of,Map.computeIfAbsent, etc.), elimination of most StringBuffer use (since string concatenation has been indified) and bring it up to at least Java 11 standards.In addition to these, I added more code tidying mostly governed by judicious evaluation of IntelliJ IDEA recommendations. Expressions have been simplified, statically known values have been respected, unused parameters and dead code removed. I did not blindly follow IDEA's recommendations; I refrained from eliminating asserts even when they're statically proven to always be true, as I like them as safeguards against future breakage. I also did not convert any switch statements into switch expressions; those have been introduced very recently in Java 14 and I'd rather give them more time. Who knows, maybe some poor soul out there plans to compile standalone Nashorn for Java 11?
All tests, including the 262 test suite, pass after these changes.
Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/nashorn pull/5/head:pull/5$ git checkout pull/5