On 15/06/2017 06:37, Mandy Chung wrote:
:
java/lang/Class.java
2453
reflectionFactory.getExecutableSharedParameterTypes(method),
reflectionFactory should be accessed via getReflectionFactory(). I see Peter
already comments on this.
Thanks, it should be getReflectionFactory().
java/lang/Module.java
901 void implAddOpensToAllUnnamed(Iterator<String> iterator) {
902 if (jdk.internal.misc.VM.isModuleSystemInited()) {
903 iterator.forEachRemaining(pn ->
904 implAddExportsOrOpens(pn, ALL_UNNAMED_MODULE, true,
true));
905 return;
906 }
AFAICT this should only be called during module system initialization.
When will this method be called after initPhase 2?
It's for use during startup (initPhase2) only. If called later then it
works as if the module somehow reflectively opened the packages to all
unnamed modules. I wouldn't object to changing it to throwing an
exception (assuming that is what you are thinking) as the JDK doesn't
have any use for this after initPhase2.
jdk/internal/loader/Loader.java
406 public Enumeration<URL> getResources(String name) throws IOException {
407 // this loader
408 List<URL> urls = findResourcesAsList(name);
409
410 // parent loader
411
parent.getResources(name).asIterator().forEachRemaining(urls::add);
412
413 return Collections.enumeration(urls);
414 }
We could consider returning an Enumeration that defers finding resources
from parent class loader after iterating the local resources.
Yes, this is possible but has a lot of potential issues. The new stream
returning resources() methods is better for doing lazy consumption and
it could override this (although still lots of potential issues, esp.
when running with a security manager, concerns about context change like
TCCL, etc.).
:
ModulePath.java
408 private static final Attributes.Name AUTOMATIC_MODULE_NAME
409 = new Attributes.Name("Automatic-Module-Name");
Should this be defined in java.util.jar.Attributes.Name?
As I recall, this was deliberately not included in the automatic name
proposal.
ServiceLoader.java
1174 } else if (!isAssignable(clazz, serviceName)) {
1175 fail(service, clazz.getName() + " not a subtype”);
Peter raises the question on isAssignable that I think it should look at the
interfaces recursively. On the other hand, I wonder if this should simply fail
if service.isAssignableFrom(clazz) returns false (which I think it’s the
existing JDK 8 behavior).
Yes, I think I will remove this.
-Alan.
