One small followup question - the test SourceLauncherTest is not
covering any shebang cases - is that deliberate? I see that those seem
to be covered in the launcher test too, but I wonder if we should have
tests for clearly broken stuff, such as
Another small question - I see that the shebang process essentially
replaces the first line separator with \n - that is probably ok given
that the file is processed internally after that - but I wonder if you
have thought about pros and cons of preserving the original line separator.
On 12/04/18 21:46, Maurizio Cimadamore wrote:
Looks great - some initial comments (I can't really comment on the
* This logic is efficient:
int magic = (in.read() << 8) + in.read();
boolean shebang = magic == (('#' << 8) + '!');
but convoluted to read; perhaps could be improved slightly by making
'#' << 8) + '!' a static constant, and comparing against that.
* I note that the reading logic in general could be simplified, e.g.
using Files.lines(Path) - but I assume that you wrote the code as is
for performance reasons (e.g. to avoid creating too many string
* I see that both the file manager and the class loader reasonably
share the same context: a Map<String, byte>. I would make this more
explicit, by having a Context class, whose state is the map, and then
have the context provide two methods:
This way the sharing will be automatic, no need to extract one field
from one place and pass it over to the other place.
* Big whohoo for being able to use the enhanced diagnostic framework
with a couple of tweaks on the makefile - I hope that would have been
the case when I put in the support, but since we have never done it -
wasn't 100% sure it would work ;-)
Overall I like it quite a lot and I think you went for a really clean
design - kudos!
On 12/04/18 21:15, Jonathan Gibbons wrote:
Please review an initial implementation for the feature described in
JEP 330: Launch Single-File Source-Code Programs.
The work is described in the JEP and CSR, and falls into various parts:
* The part to handle the new command-line options is in the native
Java launcher code.
* The part to invoke the compiler and subsequently execute the code
found in the source file is in a new class in the jdk.compiler
* There are some minor Makefile changes, to add support for a new
There are no changes to javac itself.