Hi Peter, On 9/11/2015 8:52 AM, Peter Levart wrote:
Spotted an error in "The State of the Module System" document at: http://openjdk.java.net/projects/jigsaw/spec/sotms/ ... In the "Implied readability" section, it writes:The java.sql.Driver interface, in particular, declares the public method public Logger getParentLogger(); where Logger is a type declared in the exported java.util.logging package of the java.logging module. Suppose that code in the com.foo.app module invokes this method in order to acquire a logger and log a message: String url = ...; Properties props = ...; Driver d = DriverManager.getDriver(url); Connection c = d.connect(url, props); d.getParentLogger().info("Connection acquired"); If the com.foo.app module is declared as above then this will not work: *The getParentLogger method is defined in the Logger class, which is in the java.logging module, which is not read by the com.foo.app module. That class is therefore inaccessible to code in the com.foo.app module and so the invocation of the getParentLogger method will fail, at both compile time and run time.*The correct text should be, I think: "The getParentLogger method defined in Driver class has a signature with a return type of Logger class, which is in the java.logging module, which is not read by the com.foo.app module. That class is therefore inaccessible to code in the com.foo.app module and so the invocation of the .info method will fail, at both compile time and run time (in case the modules were successfully compiled together using legacy compiler mode and then their artifacts split into modules)".
You're right.
The fact is that getParentLogger method can be invoked even though it's return type is in an unreadable module if the return type is not used in any way in such invocation. For example: d.getParentLogger().toString(); (the above assumes that .toString() is declared in Object and not overridden in Logger). If it is overriden, it can be invoked nevertheless with the following trick: ((Object) d.getParentLogger()).toString(). The important thing is that Logger as a type must not be referenced in bytecodes if the bytecodes are in a module that doesn't read java.logging module. Am I correct? I have experimented with the EA and this is my experience. Is the prototype working correctly in this respect?
You're right. The Driver::getParentLogger method IS accessible to the caller in the com.foo.app module, but the Logger type IS NOT (and so neither is the Logger::info method). Of course, the compiler can trivially prove that a cast of any class type (e.g. Logger, as given by the signature of Driver::getParentLogger) to Object is legal. Given that cast, the compiler will emit an invokevirtual Object::toString, and at run time the overriding method will be dynamically dispatched -- could be in Logger, could be in a subclass of Logger, but the caller doesn't need to know (or have access).
Now just a thought on implied readability. While it is a convenient feature, it can be dangerous if not used correctly. I think a module should declare it's own dependencies (requires) for all types it references explicitly. In above example, it invokes method Loger.info() and therefore uses types from module java.logging explicitly. A case where implied readability would be abused could be described in the following example: - java.sql module uses java.logging internally and also exposes a method Driver.getParentLogger() which returns a Logger which is declared in java.logging module. Therefore java.sql requires *public* java.logging to imply the readability of java.logging for modules that require only java.sql and not java.logging explicitly. - com.foo.internal module requires java.sql and only uses the part of it's API that doesn't need types from java.logging - com.foo.internal also uses Logger internally, but fails to declare this. It nevertheless works since java.logging is implicitly readable to it through declaration in java.sql - java.sql module decides to change it's internal use of logging API and instead of java.logging starts using org.apache.log4j. It now requires *public* org.apache.log4j instead of java.logging - an incompatible change, yes, but should not affect com.foo.internal that only uses the part of it's API that doesn't use types from java.logging - com.foo.internal now fails because it still uses java.logging, but implicit readability of it is not provided by java.sql any more. If com.foo.internal declared explicitly a dependency on java.logging, such configuration would still work. So while convenient, implied readability should be used with care.
I would hope that an IDE would raise awareness of the readability situation at step 3, where the com.foo.internal module uses Logger from a module to which it (com.foo.internal) only has implied readability.
That said, the author of the com.foo.internal module isn't doing anything wrong. He's relying on the contract offered by the java.sql module (including readability to java.logging) and is not under any obligation to use 100% of the types exported by the java.sql module.
It's the author of the java.sql module in step 4 who is potentially breaking his consumers. Per step 1, java.sql.Driver::getParentLogger has a return type from java.logging, and if that changes to come from org.apache.log4j instead, it's a source- and binary-incompatible change. Far more consumers of java.sql will break than just com.foo.internal.
Anyway, I agree with the substance of your comment, about using implied readability with care -- I recommend you send it to the EG (jpms-spec-comme...@openjdk.java.net).
Alex
