Re: 8178380: Module system implementation refresh (5/2017 update)

[Mandy Chung]

> On May 1, 2017, at 1:28 PM, Alan Bateman  wrote:
> 
>http://cr.openjdk.java.net/~alanb/8178380/2/

I reviewed all repos in the new version.

src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java

 148 if (System.getProperty("jdk.module.minimumBoot") != null) {
This property can be removed after read the value, if present.

 287 if (propValue != null && Boolean.getBoolean(propValue))

It should use Boolean.parseBoolean.

I have fixed the above injake and also added a new test to verify 
-—show-module-resolution option.

Otherwise, all looks good.
Mandy

hg: jigsaw/jake/jdk: 2 new changesets

[mandy . chung]

Changeset: f1ee71109b07
Author:mchung
Date:  2017-05-03 19:01 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/f1ee71109b07

Fix IntegrationTest.java test to check JAVA_FULL_VERSION

! test/tools/jlink/IntegrationTest.java

Changeset: 00902040a130
Author:mchung
Date:  2017-05-03 19:02 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/00902040a130

--show-module-resolution is not detected properly

! src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
+ test/tools/launcher/modules/showmoduleresolution/ShowModuleResolutionTest.java

Re: 8178380: Module system implementation refresh (5/2017 update)

[serguei.spit...@oracle.com]

Hi Alan,

I reviewed the Hotspot changes and the Jdk changes related to 
java.instrument and jdk.attch

(including a fragment in the LauncherHelper.java).

The fixes look good to me.

Thanks,
Serguei



On 5/1/17 13:28, Alan Bateman wrote:
As I mentioned in another thread, we need to get the changes 
accumulated in jake to jdk9/dev.


JDK-8178380 [1] has the summary of the changes that have accumulated 
since the last refresh.


One note on the hotspot repo is that the changes to dynamically 
augment the platform modules run into JDK-8178604 when testing with an 
exploded build. So there is a partial fix for this in jake. Harold has 
the more complete fix in review on hotspot-runtime-dev for JDK 10.


The webrevs with the changes is here:
http://cr.openjdk.java.net/~alanb/8178380/1/

-Alan

[1] https://bugs.openjdk.java.net/browse/JDK-8178380

Re: [9] Review request: 8177566: FX user module gets IllegalAccessException from sun.reflect.misc.Trampoline

[Kevin Rushforth]

Mandy Chung wrote:

Looks good.
  


Thank you for your help on this and for your review.


"Deploying an Application as a Module” section is duplicated in several
JavaBean*Property classes.  One alternative is to move it to the package
summary.  I have no objection to leave it as is. 
  


I think I'll keep it as is for the JavaBean*Property classes. Especially 
since we don't have anything currently in the package summary. There 
might be other detailed information that could go there, too.


-- Kevin




Mandy


  

On May 3, 2017, at 4:30 PM, Kevin Rushforth  wrote:

JBS: https://bugs.openjdk.java.net/browse/JDK-8177566

Here is the updated webrev with (I hope) all comments addressed:

http://cr.openjdk.java.net/~kcr/8177566/webrev.01/complete-webrev/

For those who reviewed the earlier webrev, I have prepared delta webrevs.

* Delta webrev for the fix itself (just a slight change to the error message, 
plus I hid the unused public methods in MethodUtil) :

http://cr.openjdk.java.net/~kcr/8177566/webrev.01/delta-fix-only.00/

* No changes in the tests

* Delta webrev for the doc changes:

http://cr.openjdk.java.net/~kcr/8177566/webrev.01/delta-doc-only.00/

* The sparse javadocs are also updated here:

http://cr.openjdk.java.net/~kcr/8177566/webrev.01/javadoc/

-- Kevin


Kevin Rushforth wrote:


This review is being cross-posted to both openjfx-dev and jigsaw-dev.

Please review the proposed fix for:

https://bugs.openjdk.java.net/browse/JDK-8177566
http://cr.openjdk.java.net/~kcr/8177566/webrev.00/complete-webrev/

Details of the fix as well as notes to reviewers are in the bug report [1] 
(e.g., I've also generated separate webrevs for the fix itself, the doc 
changes, and the test changes).

-- Kevin

[1] https://bugs.openjdk.java.net/browse/JDK-8177566?focusedCommentId=14074243=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14074243 

  


  

Re: [9] Review request: 8177566: FX user module gets IllegalAccessException from sun.reflect.misc.Trampoline

[Mandy Chung]

Looks good.

"Deploying an Application as a Module” section is duplicated in several
JavaBean*Property classes.  One alternative is to move it to the package
summary.  I have no objection to leave it as is. 

Mandy


> On May 3, 2017, at 4:30 PM, Kevin Rushforth  
> wrote:
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8177566
> 
> Here is the updated webrev with (I hope) all comments addressed:
> 
> http://cr.openjdk.java.net/~kcr/8177566/webrev.01/complete-webrev/
> 
> For those who reviewed the earlier webrev, I have prepared delta webrevs.
> 
> * Delta webrev for the fix itself (just a slight change to the error message, 
> plus I hid the unused public methods in MethodUtil) :
> 
> http://cr.openjdk.java.net/~kcr/8177566/webrev.01/delta-fix-only.00/
> 
> * No changes in the tests
> 
> * Delta webrev for the doc changes:
> 
> http://cr.openjdk.java.net/~kcr/8177566/webrev.01/delta-doc-only.00/
> 
> * The sparse javadocs are also updated here:
> 
> http://cr.openjdk.java.net/~kcr/8177566/webrev.01/javadoc/
> 
> -- Kevin
> 
> 
> Kevin Rushforth wrote:
>> This review is being cross-posted to both openjfx-dev and jigsaw-dev.
>> 
>> Please review the proposed fix for:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8177566
>> http://cr.openjdk.java.net/~kcr/8177566/webrev.00/complete-webrev/
>> 
>> Details of the fix as well as notes to reviewers are in the bug report [1] 
>> (e.g., I've also generated separate webrevs for the fix itself, the doc 
>> changes, and the test changes).
>> 
>> -- Kevin
>> 
>> [1] 
>> https://bugs.openjdk.java.net/browse/JDK-8177566?focusedCommentId=14074243=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14074243
>>  
>> 

Re: [9] Review request: 8177566: FX user module gets IllegalAccessException from sun.reflect.misc.Trampoline

[Kevin Rushforth]

JBS: https://bugs.openjdk.java.net/browse/JDK-8177566

Here is the updated webrev with (I hope) all comments addressed:

http://cr.openjdk.java.net/~kcr/8177566/webrev.01/complete-webrev/

For those who reviewed the earlier webrev, I have prepared delta webrevs.

* Delta webrev for the fix itself (just a slight change to the error 
message, plus I hid the unused public methods in MethodUtil) :


http://cr.openjdk.java.net/~kcr/8177566/webrev.01/delta-fix-only.00/

* No changes in the tests

* Delta webrev for the doc changes:

http://cr.openjdk.java.net/~kcr/8177566/webrev.01/delta-doc-only.00/

* The sparse javadocs are also updated here:

http://cr.openjdk.java.net/~kcr/8177566/webrev.01/javadoc/

-- Kevin


Kevin Rushforth wrote:

This review is being cross-posted to both openjfx-dev and jigsaw-dev.

Please review the proposed fix for:

https://bugs.openjdk.java.net/browse/JDK-8177566
http://cr.openjdk.java.net/~kcr/8177566/webrev.00/complete-webrev/

Details of the fix as well as notes to reviewers are in the bug report 
[1] (e.g., I've also generated separate webrevs for the fix itself, 
the doc changes, and the test changes).


-- Kevin

[1] 
https://bugs.openjdk.java.net/browse/JDK-8177566?focusedCommentId=14074243=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14074243 

Re: Java Platform Module System

[Stephan Herrmann]

Rémi,

of course there are hacks & tricks to parse such a language.
My main point is: it's outside any established concepts and technology.

And: if you happen to maintain a tool where recovery after syntax errors
and code completion and tons of other features are based on a given parser
generator, any deviation from such established concepts really hurts.
We are not in the position to exchange or rewrite the parser generator,
we have to code around it, which inevitably is a messy endeavor.

We're doing it, don't worry.

It would just be fair to spell out what kind of grammar this is.
I don't have a name for it.
The best description I could come up with so far is:
"restricted keywords are keywords when it helps to interpret them as keywords".

Stephan


On 04.05.2017 00:06, fo...@univ-mlv.fr wrote:

- Mail original -

De: "Stephan Herrmann" 
À: jigsaw-dev@openjdk.java.net, "Remi Forax" , "Alex Buckley" 

Cc: "Brian Goetz" , "Dan Smith" 

Envoyé: Mercredi 3 Mai 2017 23:31:14
Objet: Re: Java Platform Module System



On 03.05.2017 20:55, Remi Forax wrote:

It's context-free because a context free grammar defined its input in term of
terminals and the theory do not say how to map a token to a terminal.

Jay is right that it requires to use either some specific parser generator
like Tatoo [1] the one i've written 10 years ago (because i wanted the tool to
help me to extend a grammar easily) or to modify an existing parser generator so
the parser can send the production state to the lexer which will enable/disable
the automata that recognize the associated keywords .


Just feeding parser state into the Lexer doesn't cut it for Java 9,
because the classification keyword / identifier cannot be made at
the time when the stream passes the Lexer.


No, it's done between the lexer and the parser


Let me remind you of this example:
   module foo { exports transitive
How should the poor lexer recognize in this situation that transitive
is an identifier (sic) (if you complete the text accordingly)?


There is a simple solution, consider module, requires, etc as keyword in the 
lexer, and when the keyword is sent to the parser, downgrade it to an 
identifier if you are not at the right dotted production.

It's easy to implement if your lexer/parser is non blocking, i.e if you push a 
bytebuffer to the lexer and the lexer push terminals to the parser.

the other solution used by Tatoo is to instead of having one giant automata 
that recognize all tokens, works with a list automata that recognized each 
token and activate them or not depending on the parser state.


Aside from specific heuristics
(which are not available to any parser generator),


any but one :)


we only know about this classification after the parser has matched
an entire declaration.


so i suppose your parser is LR, you know the classification from the dotted 
production just before the terminal is about to be recognized.
when you construct the LR table, you know for each dotted production what are 
the terminals that can appear so the parser generator can keep these info in a 
side table and during the parsing, from the parser state, find which terminals 
can be recognized.


I'm not even sure that theory has a name for this kind of grammar.
Maybe we should speak of a constraint solver rather than a parser.


no need to have a constraint solver here, you need to export the terminals that 
will lead to a shift or a reduce for any LR states.



Stephan



Rémi



Rémi

[1] http://dl.acm.org/citation.cfm?id=1168057

- Mail original -

De: "Alex Buckley" 
À: "Jayaprakash Arthanareeswaran" , "Dan Smith"
, "Brian Goetz"

Cc: jigsaw-dev@openjdk.java.net
Envoyé: Mercredi 3 Mai 2017 19:46:54
Objet: Re: Java Platform Module System



On 5/2/2017 3:39 PM, Alex Buckley wrote:

On 5/2/2017 7:07 AM, Jayaprakash Arthanareeswaran wrote:

Chapter 2 in [1] describes context-free grammars. The addition to "3.9
Keywords" defines "restricted keywords", which prevent the grammar for
ModuleDeclaration from being context-free. This prevents compilers from
using common parser generators, since those typically only support
context-free grammars. The lexical/syntactic grammar split defined in
chapter 2 is not of much use for actual implementations of
module-info.java parsers.
The spec at least needs to point out that the given grammar for
ModuleDeclaration is not actually context-free.


The syntactic grammar in JLS8 was not context-free either; the opening
line of Chapter 2 has been false for years. For JLS9, I will remove the
claim that the lexical and syntactic grammars are context-free, and
perhaps a future JLS can discuss the difficulties in parsing the


Jan Lahoda pointed out privately that the syntactic grammar in JLS8 and
JLS9 is in 

Re: Java Platform Module System

[forax]

- Mail original -
> De: "Stephan Herrmann" 
> À: jigsaw-dev@openjdk.java.net, "Remi Forax" , "Alex 
> Buckley" 
> Cc: "Brian Goetz" , "Dan Smith" 
> 
> Envoyé: Mercredi 3 Mai 2017 23:31:14
> Objet: Re: Java Platform Module System

> On 03.05.2017 20:55, Remi Forax wrote:
> > It's context-free because a context free grammar defined its input in term 
> > of
> > terminals and the theory do not say how to map a token to a terminal.
> >
> > Jay is right that it requires to use either some specific parser generator
> > like Tatoo [1] the one i've written 10 years ago (because i wanted the tool 
> > to
> > help me to extend a grammar easily) or to modify an existing parser 
> > generator so
> > the parser can send the production state to the lexer which will 
> > enable/disable
> > the automata that recognize the associated keywords .
> 
> Just feeding parser state into the Lexer doesn't cut it for Java 9,
> because the classification keyword / identifier cannot be made at
> the time when the stream passes the Lexer.

No, it's done between the lexer and the parser

> Let me remind you of this example:
>module foo { exports transitive
> How should the poor lexer recognize in this situation that transitive
> is an identifier (sic) (if you complete the text accordingly)?

There is a simple solution, consider module, requires, etc as keyword in the 
lexer, and when the keyword is sent to the parser, downgrade it to an 
identifier if you are not at the right dotted production.

It's easy to implement if your lexer/parser is non blocking, i.e if you push a 
bytebuffer to the lexer and the lexer push terminals to the parser. 

the other solution used by Tatoo is to instead of having one giant automata 
that recognize all tokens, works with a list automata that recognized each 
token and activate them or not depending on the parser state.

> Aside from specific heuristics
> (which are not available to any parser generator),

any but one :)

> we only know about this classification after the parser has matched
> an entire declaration.

so i suppose your parser is LR, you know the classification from the dotted 
production just before the terminal is about to be recognized.
when you construct the LR table, you know for each dotted production what are 
the terminals that can appear so the parser generator can keep these info in a 
side table and during the parsing, from the parser state, find which terminals 
can be recognized.

> I'm not even sure that theory has a name for this kind of grammar.
> Maybe we should speak of a constraint solver rather than a parser.

no need to have a constraint solver here, you need to export the terminals that 
will lead to a shift or a reduce for any LR states.  

> 
> Stephan
> 

Rémi

>>
>> Rémi
>>
>> [1] http://dl.acm.org/citation.cfm?id=1168057
>>
>> - Mail original -
>>> De: "Alex Buckley" 
>>> À: "Jayaprakash Arthanareeswaran" , "Dan Smith"
>>> , "Brian Goetz"
>>> 
>>> Cc: jigsaw-dev@openjdk.java.net
>>> Envoyé: Mercredi 3 Mai 2017 19:46:54
>>> Objet: Re: Java Platform Module System
>>
>>> On 5/2/2017 3:39 PM, Alex Buckley wrote:
 On 5/2/2017 7:07 AM, Jayaprakash Arthanareeswaran wrote:
> Chapter 2 in [1] describes context-free grammars. The addition to "3.9
> Keywords" defines "restricted keywords", which prevent the grammar for
> ModuleDeclaration from being context-free. This prevents compilers from
> using common parser generators, since those typically only support
> context-free grammars. The lexical/syntactic grammar split defined in
> chapter 2 is not of much use for actual implementations of
> module-info.java parsers.
> The spec at least needs to point out that the given grammar for
> ModuleDeclaration is not actually context-free.

 The syntactic grammar in JLS8 was not context-free either; the opening
 line of Chapter 2 has been false for years. For JLS9, I will remove the
 claim that the lexical and syntactic grammars are context-free, and
 perhaps a future JLS can discuss the difficulties in parsing the
>>>
>>> Jan Lahoda pointed out privately that the syntactic grammar in JLS8 and
>>> JLS9 is in fact context-free -- it's just not LL(1). Not being LL(1) is
>>> what I should have said the grammar hasn't been for a long time.
>>>
> >> Alex

Re: Java Platform Module System

[Stephan Herrmann]

On 03.05.2017 20:55, Remi Forax wrote:
> It's context-free because a context free grammar defined its input in term of
> terminals and the theory do not say how to map a token to a terminal.
>
> Jay is right that it requires to use either some specific parser generator
> like Tatoo [1] the one i've written 10 years ago (because i wanted the tool to
> help me to extend a grammar easily) or to modify an existing parser generator 
so
> the parser can send the production state to the lexer which will 
enable/disable
> the automata that recognize the associated keywords .

Just feeding parser state into the Lexer doesn't cut it for Java 9,
because the classification keyword / identifier cannot be made at
the time when the stream passes the Lexer.
Let me remind you of this example:
   module foo { exports transitive
How should the poor lexer recognize in this situation that transitive
is an identifier (sic) (if you complete the text accordingly)?
Aside from specific heuristics
(which are not available to any parser generator),
we only know about this classification after the parser has matched
an entire declaration.
I'm not even sure that theory has a name for this kind of grammar.
Maybe we should speak of a constraint solver rather than a parser.

Stephan



Rémi

[1] http://dl.acm.org/citation.cfm?id=1168057

- Mail original -

De: "Alex Buckley" 
À: "Jayaprakash Arthanareeswaran" , "Dan Smith" 
, "Brian Goetz"

Cc: jigsaw-dev@openjdk.java.net
Envoyé: Mercredi 3 Mai 2017 19:46:54
Objet: Re: Java Platform Module System



On 5/2/2017 3:39 PM, Alex Buckley wrote:

On 5/2/2017 7:07 AM, Jayaprakash Arthanareeswaran wrote:

Chapter 2 in [1] describes context-free grammars. The addition to "3.9
Keywords" defines "restricted keywords", which prevent the grammar for
ModuleDeclaration from being context-free. This prevents compilers from
using common parser generators, since those typically only support
context-free grammars. The lexical/syntactic grammar split defined in
chapter 2 is not of much use for actual implementations of
module-info.java parsers.
The spec at least needs to point out that the given grammar for
ModuleDeclaration is not actually context-free.


The syntactic grammar in JLS8 was not context-free either; the opening
line of Chapter 2 has been false for years. For JLS9, I will remove the
claim that the lexical and syntactic grammars are context-free, and
perhaps a future JLS can discuss the difficulties in parsing the


Jan Lahoda pointed out privately that the syntactic grammar in JLS8 and
JLS9 is in fact context-free -- it's just not LL(1). Not being LL(1) is
what I should have said the grammar hasn't been for a long time.

Alex

Re: Java Platform Module System

[Remi Forax]

It's context-free because a context free grammar defined its input in term of 
terminals and the theory do not say how to map a token to a terminal.

Jay is right that it requires to use either some specific parser generator like 
Tatoo [1] the one i've written 10 years ago (because i wanted the tool to help 
me to extend a grammar easily) or to modify an existing parser generator so the 
parser can send the production state to the lexer which will enable/disable the 
automata that recognize the associated keywords .

Rémi

[1] http://dl.acm.org/citation.cfm?id=1168057

- Mail original -
> De: "Alex Buckley" 
> À: "Jayaprakash Arthanareeswaran" , "Dan Smith" 
> , "Brian Goetz"
> 
> Cc: jigsaw-dev@openjdk.java.net
> Envoyé: Mercredi 3 Mai 2017 19:46:54
> Objet: Re: Java Platform Module System

> On 5/2/2017 3:39 PM, Alex Buckley wrote:
>> On 5/2/2017 7:07 AM, Jayaprakash Arthanareeswaran wrote:
>>> Chapter 2 in [1] describes context-free grammars. The addition to "3.9
>>> Keywords" defines "restricted keywords", which prevent the grammar for
>>> ModuleDeclaration from being context-free. This prevents compilers from
>>> using common parser generators, since those typically only support
>>> context-free grammars. The lexical/syntactic grammar split defined in
>>> chapter 2 is not of much use for actual implementations of
>>> module-info.java parsers.
>>> The spec at least needs to point out that the given grammar for
>>> ModuleDeclaration is not actually context-free.
>>
>> The syntactic grammar in JLS8 was not context-free either; the opening
>> line of Chapter 2 has been false for years. For JLS9, I will remove the
>> claim that the lexical and syntactic grammars are context-free, and
>> perhaps a future JLS can discuss the difficulties in parsing the
> 
> Jan Lahoda pointed out privately that the syntactic grammar in JLS8 and
> JLS9 is in fact context-free -- it's just not LL(1). Not being LL(1) is
> what I should have said the grammar hasn't been for a long time.
> 
> Alex

Re: Java Platform Module System

[Alex Buckley]

On 5/2/2017 3:39 PM, Alex Buckley wrote:

On 5/2/2017 7:07 AM, Jayaprakash Arthanareeswaran wrote:

Chapter 2 in [1] describes context-free grammars. The addition to "3.9
Keywords" defines "restricted keywords", which prevent the grammar for
ModuleDeclaration from being context-free. This prevents compilers from
using common parser generators, since those typically only support
context-free grammars. The lexical/syntactic grammar split defined in
chapter 2 is not of much use for actual implementations of
module-info.java parsers.
The spec at least needs to point out that the given grammar for
ModuleDeclaration is not actually context-free.


The syntactic grammar in JLS8 was not context-free either; the opening
line of Chapter 2 has been false for years. For JLS9, I will remove the
claim that the lexical and syntactic grammars are context-free, and
perhaps a future JLS can discuss the difficulties in parsing the


Jan Lahoda pointed out privately that the syntactic grammar in JLS8 and 
JLS9 is in fact context-free -- it's just not LL(1). Not being LL(1) is 
what I should have said the grammar hasn't been for a long time.


Alex

hg: jigsaw/jake/jdk: Minor tweak to the api note

[mandy . chung]

Changeset: 654588131ea2
Author:mchung
Date:  2017-05-03 08:59 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/654588131ea2

Minor tweak to the api note

! src/java.base/share/classes/java/lang/ClassLoader.java

Re: [9] Review request: 8177566: FX user module gets IllegalAccessException from sun.reflect.misc.Trampoline

[Kevin Rushforth]

Forgot to respond to this:


Also in MethodUtil::invoke
  61if (!module.isExported(packageName)) {
You can do this check only if module.isNamed. 


No, this check will work fine on an unnamed module. Module::isExported 
and Module::isOpen always return true for Module::unnamed.


-- Kevin



Kevin Rushforth wrote:
OK, I'll create a more informative message. I think it will be more 
clear in the message to just say that it needs to "open" the package 
to javafx.base, since that would be the recommendation for a package 
that isn't already exported unconditionally. I'll send out a new 
webrev this morning with all feedback incorporated.


-- Kevin


Mandy Chung wrote:
On May 2, 2017, at 2:22 PM, Kevin Rushforth 
 wrote:


Here is the message:

IllegalAccessException: class com.sun.javafx.property.MethodHelper 
cannot access class com.foo (in module foo.app) because module 
foo.app does not open com.foo to javafx.base



It would be better to emit a more informative message e.g. 
“javafx.base cannot access class com.foo (in module foo.app).  Either 
exports com.foo unqualifiedly or open com.foo to javafx.base”.  Also 
in MethodUtil::invoke

  61if (!module.isExported(packageName)) {
You can do this check only if module.isNamed.

 
It is roughly the same message that any similar illegal access would 
generate (e.g., we get similar error messages when FXML tries to 
call setAccessible for a field annotated with @FXML if the module is 
not "open" to javafx.fxml).


   

javafx.base/src/main/java/com/sun/javafx/property/MethodHelper.java
javafx.fxml/src/main/java/com/sun/javafx/fxml/MethodHelper.java
javafx.web/src/main/java/com/sun/webkit/MethodHelper.java
  45 public static Object invoke(Method m, Object obj, Object[] 
params)


To avoid 3 ModuleHelper classes, the invoke method can take
the callerModule argument to replace this line:   56 final 
Module thisModule = MethodHelper.class.getModule();

I'm fairly certain that won't work. Module::addOpens is caller 
sensitive and will only work when called from the module in question.





You are right.  Wont’t work.
 

javafx.base/src/main/java/com/sun/javafx/reflect/MethodUtil.java
  There are a few other public methods which I think JavaFX doesn’t
  need and can be removed.

Yes, I could do this to reduce the public footprint of the class. To 
minimize the diffs between the original and our copy, I might just 
comment out the "public". That would also make it easier to add them 
back in a future version (e.g., to eventually get rid of all 
dependency on sun.reflect.misc). Thoughts?



I will leave it up to you.
Mandy

  

Re: [9] Review request: 8177566: FX user module gets IllegalAccessException from sun.reflect.misc.Trampoline

[Kevin Rushforth]

OK, I'll create a more informative message. I think it will be more 
clear in the message to just say that it needs to "open" the package to 
javafx.base, since that would be the recommendation for a package that 
isn't already exported unconditionally. I'll send out a new webrev this 
morning with all feedback incorporated.


-- Kevin


Mandy Chung wrote:

On May 2, 2017, at 2:22 PM, Kevin Rushforth  wrote:

Here is the message:

IllegalAccessException: class com.sun.javafx.property.MethodHelper cannot access class com.foo (in module foo.app) because module foo.app does not open com.foo to javafx.base 




It would be better to emit a more informative message e.g. “javafx.base cannot 
access class com.foo (in module foo.app).  Either exports com.foo unqualifiedly 
or open com.foo to javafx.base”.  Also in MethodUtil::invoke
  61if (!module.isExported(packageName)) {
You can do this check only if module.isNamed.

  

It is roughly the same message that any similar illegal access would generate (e.g., we 
get similar error messages when FXML tries to call setAccessible for a field annotated 
with @FXML if the module is not "open" to javafx.fxml).



javafx.base/src/main/java/com/sun/javafx/property/MethodHelper.java
javafx.fxml/src/main/java/com/sun/javafx/fxml/MethodHelper.java
javafx.web/src/main/java/com/sun/webkit/MethodHelper.java
  45 public static Object invoke(Method m, Object obj, Object[] params)

To avoid 3 ModuleHelper classes, the invoke method can take
the callerModule argument to replace this line: 
  56 final Module thisModule = MethodHelper.class.getModule();
  
  

I'm fairly certain that won't work. Module::addOpens is caller sensitive and 
will only work when called from the module in question.




You are right.  Wont’t work.
  

javafx.base/src/main/java/com/sun/javafx/reflect/MethodUtil.java
  There are a few other public methods which I think JavaFX doesn’t
  need and can be removed.
  
  

Yes, I could do this to reduce the public footprint of the class. To minimize the diffs 
between the original and our copy, I might just comment out the "public". That 
would also make it easier to add them back in a future version (e.g., to eventually get 
rid of all dependency on sun.reflect.misc). Thoughts?



I will leave it up to you.
Mandy

  

Re: 8178380: Module system implementation refresh (5/2017 update)

[harold seigel]

Hi Alan,

The hotspot changes look good.

Thanks, Harold


On 5/1/2017 4:28 PM, Alan Bateman wrote:
As I mentioned in another thread, we need to get the changes 
accumulated in jake to jdk9/dev.


JDK-8178380 [1] has the summary of the changes that have accumulated 
since the last refresh.


One note on the hotspot repo is that the changes to dynamically 
augment the platform modules run into JDK-8178604 when testing with an 
exploded build. So there is a partial fix for this in jake. Harold has 
the more complete fix in review on hotspot-runtime-dev for JDK 10.


The webrevs with the changes is here:
http://cr.openjdk.java.net/~alanb/8178380/1/

-Alan

[1] https://bugs.openjdk.java.net/browse/JDK-8178380