8344706: Compiler Implementation of Compact Source Files and Instance Main Methods #24438

lahodaj · 2025-04-04T07:20:23Z

This is a PR that implements JEP: Compact Source Files and Instance Main Methods. Changes include:

java.io.IO moved to java.lang.IO, and no longer uses System.console() to implement the methods (thanks to @stuart-marks)
java. ... .IO is no longer automatically imported in any compilation unit
the feature is finalized (i.e. no longer requires --enable-preview)

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires CSR request JDK-8353437 to be approved
Change requires a JEP request to be targeted

Integration blocker

⚠️ Title mismatch between PR and JBS for issue JDK-8344706

Issues

JDK-8344706: Implement JEP 512: Compact Source Files and Instance Main Methods (Sub-task - P4) ⚠️ Title mismatch between PR and JBS.
JDK-8344699: JEP 512: Compact Source Files and Instance Main Methods (JEP)
JDK-8353437: Implement JEP 512: Compact Source Files and Instance Main Methods (CSR)

Reviewers

Christian Stein (@sormuras - Committer) Review applies to a5d223e1
Vicente Romero (@vicente-romero-oracle - Reviewer) Review applies to 05cc336e
Naoto Sato (@naotoj - Reviewer)

Contributors

Stuart Marks <smarks@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24438/head:pull/24438
$ git checkout pull/24438

Update a local copy of the PR:
$ git checkout pull/24438
$ git pull https://git.openjdk.org/jdk.git pull/24438/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24438

View PR using the GUI difftool:
$ git pr show -t 24438

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24438.diff

Using Webrev

Link to Webrev Comment

…spec. Augment class spec to discuss buffering of bytes from System.in. Change "terminates the line" with "writes a line separator".

…concise-source-files

bridgekeeper · 2025-04-04T07:21:15Z

👋 Welcome back jlahoda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-04-04T07:21:42Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-04-04T07:22:06Z

⚠️ @lahodaj This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

lahodaj · 2025-04-04T07:22:12Z

/contributor add @stuart-marks

lahodaj · 2025-04-04T07:22:43Z

/jep JDK-8344699

openjdk · 2025-04-04T08:56:00Z

@lahodaj
Contributor Stuart Marks <smarks@openjdk.org> successfully added.

openjdk · 2025-04-04T08:56:28Z

@lahodaj
This pull request will not be integrated until the JEP JDK-8344699 has been targeted.

test/jdk/java/io/IO/IO.java

lukellmann · 2025-04-05T00:55:55Z

src/java.base/share/classes/java/lang/IO.java

+        System.out.print(obj);
+        System.out.flush();


Is it worth using a local variable to avoid calling print and flush on different streams in case System.out is reassigned in between?

var out = System.out; out.print(obj); out.flush();

This code is not that perf sensitive, guess it is fine as-is

I think there's a correctness angle. If you print something to a stream you want to flush the same stream. If System.out is changed in between (unlikely, but possible) then output destined for the old stream could be stranded in its buffer.

Updated as suggested; thanks @lukellmann .

lukellmann · 2025-04-05T00:58:44Z

src/java.base/share/classes/java/lang/IO.java

+     * @throws IOError if an I/O error occurs
+     */
+    public static String readln() {
+        try {
+            return reader().readLine();
+        } catch (IOException ioe) {
+            throw new IOError(ioe);
+        }


Was UncheckedIOException considered? It might be better for beginners to learn throwing exceptions instead of errors.

This implementation is not to be emulated by beginners. If an Exception.is thrown here, a user might be tempted to add handlers in their code, while this is an issue with the setup.

Someone is bound to ask why the readln method throw but the println methods don't.

The IOError was carried over from Console.readLine(), which throws IOError on error. Of course we're decoupled from Console now, but that was the original reason.

My guess is that Console methods throw IOError because (a) they didn't want to make everybody catch IOException, (b) they didn't want to maintain an internal error state like PrintStream, and (c) they wanted to throw a throwable whose type conveyed the meaning that it wasn't expected to be handled. IOError and Console were both added in JDK 1.6. Maybe that reasoning still applies here.

UncheckedIOException wasn't added until JDK 1.8. Note also that UncheckedIOException has a cause of IOException, so it can't arise from some other throwable. This was used in the previous implementation, where IOError was thrown if Console was null. That doesn't apply anymore, but IOError is still somewhat more flexible than UncheckedIOException in this regard.

I think we need to say something, implicitly or explicitly, about error handling. Note that PrintStream has this internal error state so output is covered already. For input it seems like IOError is reasonable, but maybe there's a better choice.

The readln methods handle malformed-input and unmappable-character errors by dropping, and using a replacement value. So erroneous input doesn't throw IOError with a CharacterCodingException as the cause.

System.in.close() would a be wild thing to do, but a case where readln would throw IOError.

Ah. Does the specification here need to state the policy around malformed input and unmappable characters? (Or, maybe in the class specification.)

People have ended up in cases where System.in is closed. It can occur when combining use of Scanner to read from System.in and applying the anything-that-you-open-must-be-closed rule, e.g.,

try (Scanner sc = new Scanner(System.in)) { String s = sc.nextLine(); }

which is a mistake, but it takes a long time to explain....

I added a clause to the class specification that covers malformed/unmappable byte sequences.

mlbridge · 2025-04-07T07:01:32Z

Webrevs

sormuras

Changes in source launcher related tests look good.

AlanBateman · 2025-04-07T09:12:27Z

src/java.base/share/classes/java/lang/IO.java

+ * If this property is not present, or if the charset it names cannot be loaded, then
+ * UTF-8 is used instead. These internal objects are created upon the first call to
+ * either of the {@code readln} methods and are stored for subsequent reuse by these
+ * methods.


@stuart-marks Can we rephrase this paragraph so that it doesn't use phrase "internal objects"? The class does speak of buffering and how it might impact code that mixes use of System.in and IO.readln so I agree with that part it's just the "internal objects" phrase that is confusing to read in this class.

I used "internal objects" because I wanted to avoid naming concrete classes, which might or might not be used. Previous drafts mentioned BufferedReader, InputStreamReader, and CharsetDecoder.

I could replace "internal objects" with something more descriptive like "objects to handle buffering and charset decoding" but I'd still need a noun phrase to refer to them later. Maybe "buffering and decoding objects" ?

Can we rephrase the 1st sentence to read:
"The readln() and readln(String) methods in this class decode bytes read from System.in into characters."

And remove the last "these internal objects" sentence.

And for the next paragraph, change the first line to:
"The readln methods may buffer additional bytes..."

So we remove references to "internal objects."

I'm in the midst of rewriting all of this stuff. The main concern here is that applications can use System.in more or less as they please (presumably without buffering) until the first call to a readln method, at which time something happens and effectively the application cannot use System.in anymore. I'll post the revision when I've finished it.

naotoj · 2025-04-07T17:58:45Z

test/jdk/java/lang/IO/IO/IO.java

Is there reason to create a test subdirectory IO under java.lang.IO package?

Sorry, I missed this comment. Fixed here:
bacc1a2

naotoj · 2025-04-07T18:05:41Z

test/jdk/java/lang/IO/IO/IO.java

@@ -51,7 +51,7 @@
 /*
 * @test
 * @bug 8305457 8342936 8351435


Issue id needs to be added (and other test files too)

Sorry, I missed this comment. Fixed here:
5d02ff6

vicente-romero-oracle

javac compiler related code and tests lgtm

…jects."

AlanBateman · 2025-04-09T10:22:05Z

src/java.base/share/classes/java/lang/IO.java

+ * Charset decoding is set up upon the first call to one of the {@code readln} methods.
+ * Decoding may buffer additional bytes beyond those that have been decoded to characters
+ * returned to the application. After the first call to one of the {@code readln} methods,
+ * any subsequent use of {@code System.in} results in unspecified behavior.


I wonder if we should add a note about mixing API usage into System.in too. There are tutorials and books that show examples build on System.in that will add buffering on that input source.

Might be a good idea. I'll handle this separately. I don't want it to refer to a Preview API. I'll probably just use Scanner and InputStreamReader as examples. (Oh wait, this supposedly isn't a Preview API anymore...)

AlanBateman · 2025-04-09T10:46:40Z

src/java.base/share/classes/java/lang/IO.java

+ * The {@link #readln()} and {@link #readln(String)} methods decode bytes read from
+ * {@code System.in} into characters. The charset used for decoding is specified by the
+ * {@link System#getProperties stdin.encoding} property. If this property is not present,
+ * or if the charset it names cannot be loaded, then UTF-8 is used instead.


stdout.encoding and stderr.encoding are specified (in System.getProperties) to lead to unspecified behavior if started with either property set to a value other than UTF-8. We should work through the issues of introducing stdin.encoding as soon as we can, esp. the redirect cases and whether there is use cases for setting any of these properties on the command line.

Yes, believe it or not, I am still studying this...

vicente-romero-oracle

lgtm

AlanBateman · 2025-04-22T08:14:23Z

src/java.base/share/classes/java/lang/IO.java

+ * <p>
+ * The {@link #readln()} and {@link #readln(String)} methods decode bytes read from
+ * {@code System.in} into characters. The charset used for decoding is specified by the
+ * {@link System#getProperties stdin.encoding} property. If this property is not present,


@stuart-marks Are you planning to propose/integrate pull/24738 in advance of the JEP update? Asking because this paragraph will need to be adjusted if pull/24738 doesn't go in first.

Depends on how close you think 24738 is to being ready. As it stands it seems ok; but I'm still looking at the potential impact on other part of the JDK. In particular are there other JDK APIs that should be adjusted to mention stdin.encoding?

As for the impact here, if 24738 doesn't go in first, then I'm not quite sure what this should say. I guess it could say "default charset" or something and then it could be amended with a bugfix later.

AlanBateman · 2025-04-22T08:34:36Z

src/java.base/share/classes/jdk/internal/misc/MethodFinder.java

-        if (isPreview && mainMethod == null) {
+        if (mainMethod == null) {
+            //if not public method, try to lookup a non-public one
+            mainMethod = JLA.findMethod(cls, false, "main", String[].class);


Is there a reason why the first findMethod can't use publicOnly=false?

My reason was that when publicOnly=false, there's more classloading happening (unsurprisingly, as also the non-public methods must be resolved). Which may lead to a change of behavior if the traditional public static main(String[]) exists.

IIRC there is a test relying on this behavior, but I don't have the name offhand. Will put it here when I have it.

For example:

$ cat Main.java public class Main { public static void main(String... args) { System.err.println("Hello!"); } private Missing get() { return null; } } $ cat Missing.java public class Missing { } $ jdk-24/bin/javac Main.java Missing.java; rm Missing.class $ jdk-24/bin/java -classpath . Main Hello! $ jdk-without-publicOnly=true/bin/java -classpath . Main Error: Unable to initialize main class Main Caused by: java.lang.NoClassDefFoundError: Missing

The tests that would be affected by the extra classloading appear to be:

test/jdk/java/lang/System/SecurityManagerWarnings.java test/jdk/java/lang/System/LoggerFinder/SignedLoggerFinderTest/SignedLoggerFinderTest.java

AlanBateman · 2025-04-22T08:35:20Z

src/java.base/share/classes/java/io/Console.java

@@ -148,100 +147,6 @@ public Reader reader() {
        throw newUnsupportedOperationException();
    }

-    /**


The copyright header will need to be updated on the changes to java.base as they seem to be first update to these sources in 2025.

Fixed here:
44fccf3

This reverts commit 7047cf0.

…nto finalize-concise-source-files

naotoj

LGTM. I guess the coordination with Stuart is underway

openjdk · 2025-04-23T13:29:50Z

@lahodaj this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout finalize-concise-source-files
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

stuart-marks and others added 21 commits January 13, 2025 19:41

Initial cut at java.lang.IO (from 2024-12-19).

6e9e093

Merge branch 'master' into JavaLangIO

Loading
Loading status checks…

e801c97

Merge remote-tracking branch 'upstream/master' into JavaLangIO

7c98234

Revise and strip down java.lang.IO

4ebd601

Various tweaks to charset decoding and line reading.

Loading
Loading status checks…

fb80e69

Updatiing javac and related tests to changes in java.lang.IO.

2c5ff02

Merge branch 'master' into JavaLangIO

276fdfa

Remove autoflush, and add explicit flushing only within print().

Loading
Loading status checks…

2f0dfc9

Code to finalize compact source files.

5c7ee00

More cleanup.

0177b4c

Fixing tests.

bc2d958

Merge branch 'JavaLangIO' into finalize-concise-source-files

7d18bdc

Adjusting tests.

7997850

More test fixes.

39a28ea

Removing preview flag from new main method handling.

643083d

Merge branch 'master' into finalize-concise-source-files

5cb7ffa

Merge branch 'master' into JavaLangIO

0dba1c9

Clean up some charset stuff, use stdin.encoding property.

Loading
Loading status checks…

c2b3637

Responses to Alan's comments: clean up readln() and align with class …

Loading
Loading status checks…

7bacc29

…spec. Augment class spec to discuss buffering of bytes from System.in. Change "terminates the line" with "writes a line separator".

Merge remote-tracking branch 'stuart-marks/JavaLangIO' into finalize-…

9f45f29

…concise-source-files

Merge branch 'master' into finalize-concise-source-files

Loading
Loading status checks…

6b88025

openjdk bot added the csr label Apr 4, 2025

Removing unnecessary enable preview from tests.

Loading
Loading status checks…

3d181d9

liach reviewed Apr 5, 2025

View reviewed changes

test/jdk/java/io/IO/IO.java Outdated Show resolved Hide resolved

lukellmann reviewed Apr 5, 2025

View reviewed changes

Moving tests as suggested, removing unused import.

Loading
Loading status checks…

a5d223e

lahodaj marked this pull request as ready for review April 7, 2025 06:57

openjdk bot added the rfr label Apr 7, 2025

sormuras approved these changes Apr 7, 2025

View reviewed changes

AlanBateman reviewed Apr 7, 2025

View reviewed changes

Use local variable for System.out in print().

Loading
Loading status checks…

be8846e

naotoj reviewed Apr 7, 2025

View reviewed changes

vicente-romero-oracle approved these changes Apr 7, 2025

View reviewed changes

Rewrite bits about charset decoding, removing mention of "internal ob…

Loading
Loading status checks…

33e280d

…jects."

AlanBateman reviewed Apr 9, 2025

View reviewed changes

Add clause about handling of malformed/unmappable bytes.

Loading
Loading status checks…

05cc336

openjdk bot mentioned this pull request Apr 11, 2025

8320255: Add method to java.lang.Class to find main method #24594

Draft

4 tasks

vicente-romero-oracle approved these changes Apr 21, 2025

View reviewed changes

AlanBateman reviewed Apr 22, 2025

View reviewed changes

lahodaj added 7 commits April 22, 2025 11:36

Scratch

7047cf0

Fixing IO tests path

bacc1a2

Adding bug number to tests.

5d02ff6

Updating copyright year.

44fccf3

Fixing tests.

7ec3b5c

Reverting test commit.

a4cdcab

This reverts commit 7047cf0.

Merge remote-tracking branch 'origin/finalize-concise-source-files' i…

Loading
Loading status checks…

4839988

…nto finalize-concise-source-files

naotoj approved these changes Apr 22, 2025

View reviewed changes

openjdk bot added the merge-conflict label Apr 23, 2025

8344706: Compiler Implementation of Compact Source Files and Instance Main Methods #24438

Are you sure you want to change the base?

8344706: Compiler Implementation of Compact Source Files and Instance Main Methods #24438

Conversation

lahodaj commented Apr 4, 2025 • edited by openjdk bot Loading

Progress

Integration blocker

Issues

Reviewers

Contributors

Reviewing

bridgekeeper bot commented Apr 4, 2025

openjdk bot commented Apr 4, 2025

openjdk bot commented Apr 4, 2025

lahodaj commented Apr 4, 2025

lahodaj commented Apr 4, 2025

openjdk bot commented Apr 4, 2025

openjdk bot commented Apr 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuart-marks Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

mlbridge bot commented Apr 7, 2025 • edited Loading

Webrevs

sormuras left a comment

Choose a reason for hiding this comment

AlanBateman Apr 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vicente-romero-oracle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuart-marks Apr 9, 2025 • edited Loading

Choose a reason for hiding this comment

AlanBateman Apr 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vicente-romero-oracle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naotoj left a comment

Choose a reason for hiding this comment

openjdk bot commented Apr 23, 2025

lahodaj commented Apr 4, 2025 •

edited by openjdk bot

Loading

stuart-marks Apr 10, 2025 •

edited

Loading

mlbridge bot commented Apr 7, 2025 •

edited

Loading

AlanBateman Apr 7, 2025 •

edited

Loading

stuart-marks Apr 9, 2025 •

edited

Loading

AlanBateman Apr 9, 2025 •

edited

Loading