JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

jan.lahoda
Hello,

There is:
https://bugs.openjdk.java.net/browse/JDK-8153362

which is about a new warning that should be produced by javac when
exported API refers to types not exported/accessible to the API clients.

I've put my current javac change here:
http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/

There are some warnings produced for existing code in the hotspot and
jdk repositories (see the end for the warnings), I was using these
patches to keep JDK buildable:
hotspot repository:
http://cr.openjdk.java.net/~jlahoda/8153362/hotspot.00/
jdk repository:
http://cr.openjdk.java.net/~jlahoda/8153362/jdk.00/

Any help with those (esp. those in the hotspot repository) would be
wholeheartedly welcome.

Any feedback on this is welcome!

Thanks,
    Jan

The warnings:
-hotspot repository:
.../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.runtime/src/jdk/vm/ci/runtime/services/JVMCICompilerFactory.java:72:
warning: unexported type referenced in exported API
     public abstract JVMCICompiler createCompiler(JVMCIRuntime runtime);
                     ^
.../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.runtime/src/jdk/vm/ci/runtime/services/JVMCICompilerFactory.java:72:
warning: unexported type referenced in exported API
     public abstract JVMCICompiler createCompiler(JVMCIRuntime runtime);
                                                  ^
error: warnings found and -Werror specified
.../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotJVMCICompilerFactory.java:54:
warning: unexported type referenced in exported API
     public int getCompilationLevelAdjustment(HotSpotVMConfig config) {
                                              ^
.../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotJVMCICompilerFactory.java:73:
warning: unexported type referenced in exported API
     public int adjustCompilationLevel(HotSpotVMConfig config, Class<?>
declaringClass, String name, String signature, boolean isOsr, int level) {
                                       ^
.../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
warning: unexported type referenced in exported API
     public void notifyInstall(HotSpotCodeCacheProvider
hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
compiledCode) {
                               ^
.../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
warning: unexported type referenced in exported API
     public void notifyInstall(HotSpotCodeCacheProvider
hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
compiledCode) {

           ^
.../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
warning: unexported type referenced in exported API
     public void notifyInstall(HotSpotCodeCacheProvider
hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
compiledCode) {

                                        ^
1 error
7 warnings

-jdk/java.desktop:
.../jdk/src/java.desktop/share/classes/javax/swing/JRootPane.java:333:
warning: inaccessible type referenced in exported API
     protected DefaultAction defaultPressAction;
               ^
.../jdk/src/java.desktop/share/classes/javax/swing/JRootPane.java:344:
warning: inaccessible type referenced in exported API
     protected DefaultAction defaultReleaseAction;
               ^
error: warnings found and -Werror specified
.../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java:742:
warning: inaccessible type referenced in exported API
         protected MetalBumps bumps = new MetalBumps( 10, 10,
                   ^
.../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalFileChooserUI.java:926:
warning: inaccessible type referenced in exported API
     protected DirectoryComboBoxRenderer
createDirectoryComboBoxRenderer(JFileChooser fc) {
               ^
.../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalScrollBarUI.java:65:
warning: inaccessible type referenced in exported API
     protected MetalBumps bumps;
               ^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
1 error
5 warnings


-jdk/java.naming:
.../jdk/src/java.naming/share/classes/javax/naming/CompoundName.java:156: warning:
inaccessible type referenced in exported API
     protected transient NameImpl impl;
                         ^
error: warnings found and -Werror specified
1 error
1 warning


-jdk/jdk.accessibility:
.../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/AWTEventMonitor.java:219:
warning: inaccessible type referenced in exported API
     static protected AWTEventsListener awtListener = new
AWTEventsListener();
                      ^
error: warnings found and -Werror specified
.../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/AccessibilityEventMonitor.java:66:
warning: inaccessible type referenced in exported API
     static protected final AccessibilityEventListener
accessibilityListener =
                            ^
.../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/SwingEventMonitor.java:74:
warning: inaccessible type referenced in exported API
     static protected final SwingEventListener swingListener = new
SwingEventListener();
                            ^
1 error
3 warnings


-jdk/jdk.jsobject:
.../jdk/src/jdk.jsobject/share/classes/netscape/javascript/JSObject.java:153:
warning: type from a module referenced in exported API that is not
requires public from this module
     public static JSObject getWindow(Applet applet) throws JSException {
                                      ^
error: warnings found and -Werror specified
1 error
1 warning

-jdk/jdk.security.jgss:
.../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSContext.java:42:
warning: type that is invisible for some modules which read this package
referenced in exported API
     static class ExtendedGSSContextImpl extends GSSContextImpl
                                                 ^
.../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSContext.java:45:
warning: type that is invisible for some modules which read this package
referenced in exported API
         public ExtendedGSSContextImpl(GSSContextImpl old) {
                                       ^
error: warnings found and -Werror specified
.../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSCredential.java:38:
warning: type that is invisible for some modules which read this package
referenced in exported API
     static class ExtendedGSSCredentialImpl extends GSSCredentialImpl
                                                    ^
.../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSCredential.java:41:
warning: type that is invisible for some modules which read this package
referenced in exported API
         public ExtendedGSSCredentialImpl(GSSCredentialImpl old) {
                                          ^
1 error
4 warnings
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

Phil Race
Hmm .. given that the majority of the jdk changes are in client -
specifically
swing & accessibility -  including the swing mailing list would have
increased the
likelihood of the right people clicking on this webrev link.

IMO,  we should remove these unusable fields from the Swing API - where
we can - and I think some we can, and really it ought to be all.

So I suggest that you leave alone those files and submit
bugs against swing & accessibility and the area owner
can then make the decision as to the appropriate treatment.

You can keep the JDK buildable in the meanwhile by suppressing the
lint warnings on the desktop module.

-phil.


On 06/13/2016 09:12 AM, Jan Lahoda wrote:

> Hello,
>
> There is:
> https://bugs.openjdk.java.net/browse/JDK-8153362
>
> which is about a new warning that should be produced by javac when
> exported API refers to types not exported/accessible to the API clients.
>
> I've put my current javac change here:
> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
>
> There are some warnings produced for existing code in the hotspot and
> jdk repositories (see the end for the warnings), I was using these
> patches to keep JDK buildable:
> hotspot repository:
> http://cr.openjdk.java.net/~jlahoda/8153362/hotspot.00/
> jdk repository:
> http://cr.openjdk.java.net/~jlahoda/8153362/jdk.00/
>
> Any help with those (esp. those in the hotspot repository) would be
> wholeheartedly welcome.
>
> Any feedback on this is welcome!
>
> Thanks,
>    Jan
>
> The warnings:
> -hotspot repository:
> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.runtime/src/jdk/vm/ci/runtime/services/JVMCICompilerFactory.java:72:
> warning: unexported type referenced in exported API
>     public abstract JVMCICompiler createCompiler(JVMCIRuntime runtime);
>                     ^
> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.runtime/src/jdk/vm/ci/runtime/services/JVMCICompilerFactory.java:72:
> warning: unexported type referenced in exported API
>     public abstract JVMCICompiler createCompiler(JVMCIRuntime runtime);
>                                                  ^
> error: warnings found and -Werror specified
> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotJVMCICompilerFactory.java:54:
> warning: unexported type referenced in exported API
>     public int getCompilationLevelAdjustment(HotSpotVMConfig config) {
>                                              ^
> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotJVMCICompilerFactory.java:73:
> warning: unexported type referenced in exported API
>     public int adjustCompilationLevel(HotSpotVMConfig config, Class<?>
> declaringClass, String name, String signature, boolean isOsr, int
> level) {
>                                       ^
> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
> warning: unexported type referenced in exported API
>     public void notifyInstall(HotSpotCodeCacheProvider
> hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
> compiledCode) {
>                               ^
> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
> warning: unexported type referenced in exported API
>     public void notifyInstall(HotSpotCodeCacheProvider
> hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
> compiledCode) {
>
>           ^
> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
> warning: unexported type referenced in exported API
>     public void notifyInstall(HotSpotCodeCacheProvider
> hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
> compiledCode) {
>
>                                        ^
> 1 error
> 7 warnings
>
> -jdk/java.desktop:
> .../jdk/src/java.desktop/share/classes/javax/swing/JRootPane.java:333:
> warning: inaccessible type referenced in exported API
>     protected DefaultAction defaultPressAction;
>               ^
> .../jdk/src/java.desktop/share/classes/javax/swing/JRootPane.java:344:
> warning: inaccessible type referenced in exported API
>     protected DefaultAction defaultReleaseAction;
>               ^
> error: warnings found and -Werror specified
> .../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java:742:
> warning: inaccessible type referenced in exported API
>         protected MetalBumps bumps = new MetalBumps( 10, 10,
>                   ^
> .../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalFileChooserUI.java:926:
> warning: inaccessible type referenced in exported API
>     protected DirectoryComboBoxRenderer
> createDirectoryComboBoxRenderer(JFileChooser fc) {
>               ^
> .../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalScrollBarUI.java:65:
> warning: inaccessible type referenced in exported API
>     protected MetalBumps bumps;
>               ^
> Note: Some input files use or override a deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> 1 error
> 5 warnings
>
>
> -jdk/java.naming:
> .../jdk/src/java.naming/share/classes/javax/naming/CompoundName.java:156:
> warning: inaccessible type referenced in exported API
>     protected transient NameImpl impl;
>                         ^
> error: warnings found and -Werror specified
> 1 error
> 1 warning
>
>
> -jdk/jdk.accessibility:
> .../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/AWTEventMonitor.java:219:
> warning: inaccessible type referenced in exported API
>     static protected AWTEventsListener awtListener = new
> AWTEventsListener();
>                      ^
> error: warnings found and -Werror specified
> .../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/AccessibilityEventMonitor.java:66:
> warning: inaccessible type referenced in exported API
>     static protected final AccessibilityEventListener
> accessibilityListener =
>                            ^
> .../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/SwingEventMonitor.java:74:
> warning: inaccessible type referenced in exported API
>     static protected final SwingEventListener swingListener = new
> SwingEventListener();
>                            ^
> 1 error
> 3 warnings
>
>
> -jdk/jdk.jsobject:
> .../jdk/src/jdk.jsobject/share/classes/netscape/javascript/JSObject.java:153:
> warning: type from a module referenced in exported API that is not
> requires public from this module
>     public static JSObject getWindow(Applet applet) throws JSException {
>                                      ^
> error: warnings found and -Werror specified
> 1 error
> 1 warning
>
> -jdk/jdk.security.jgss:
> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSContext.java:42:
> warning: type that is invisible for some modules which read this
> package referenced in exported API
>     static class ExtendedGSSContextImpl extends GSSContextImpl
>                                                 ^
> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSContext.java:45:
> warning: type that is invisible for some modules which read this
> package referenced in exported API
>         public ExtendedGSSContextImpl(GSSContextImpl old) {
>                                       ^
> error: warnings found and -Werror specified
> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSCredential.java:38:
> warning: type that is invisible for some modules which read this
> package referenced in exported API
>     static class ExtendedGSSCredentialImpl extends GSSCredentialImpl
>                                                    ^
> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSCredential.java:41:
> warning: type that is invisible for some modules which read this
> package referenced in exported API
>         public ExtendedGSSCredentialImpl(GSSCredentialImpl old) {
>                                          ^
> 1 error
> 4 warnings

Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

Phil Race
PS .. also you probably should  just suppress lint on the jdk.jsobject
module

The change you propose to JSObject is going to cause a potential conflict
with the ongoing review discussion about this here :-

http://mail.openjdk.java.net/pipermail/awt-dev/2016-June/011472.html

-phil.

On 6/13/16, 10:59 AM, Phil Race wrote:

> Hmm .. given that the majority of the jdk changes are in client -
> specifically
> swing & accessibility -  including the swing mailing list would have
> increased the
> likelihood of the right people clicking on this webrev link.
>
> IMO,  we should remove these unusable fields from the Swing API - where
> we can - and I think some we can, and really it ought to be all.
>
> So I suggest that you leave alone those files and submit
> bugs against swing & accessibility and the area owner
> can then make the decision as to the appropriate treatment.
>
> You can keep the JDK buildable in the meanwhile by suppressing the
> lint warnings on the desktop module.
>
> -phil.
>
>
> On 06/13/2016 09:12 AM, Jan Lahoda wrote:
>> Hello,
>>
>> There is:
>> https://bugs.openjdk.java.net/browse/JDK-8153362
>>
>> which is about a new warning that should be produced by javac when
>> exported API refers to types not exported/accessible to the API clients.
>>
>> I've put my current javac change here:
>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
>>
>> There are some warnings produced for existing code in the hotspot and
>> jdk repositories (see the end for the warnings), I was using these
>> patches to keep JDK buildable:
>> hotspot repository:
>> http://cr.openjdk.java.net/~jlahoda/8153362/hotspot.00/
>> jdk repository:
>> http://cr.openjdk.java.net/~jlahoda/8153362/jdk.00/
>>
>> Any help with those (esp. those in the hotspot repository) would be
>> wholeheartedly welcome.
>>
>> Any feedback on this is welcome!
>>
>> Thanks,
>>    Jan
>>
>> The warnings:
>> -hotspot repository:
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.runtime/src/jdk/vm/ci/runtime/services/JVMCICompilerFactory.java:72:
>> warning: unexported type referenced in exported API
>>     public abstract JVMCICompiler createCompiler(JVMCIRuntime runtime);
>>                     ^
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.runtime/src/jdk/vm/ci/runtime/services/JVMCICompilerFactory.java:72:
>> warning: unexported type referenced in exported API
>>     public abstract JVMCICompiler createCompiler(JVMCIRuntime runtime);
>>                                                  ^
>> error: warnings found and -Werror specified
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotJVMCICompilerFactory.java:54:
>> warning: unexported type referenced in exported API
>>     public int getCompilationLevelAdjustment(HotSpotVMConfig config) {
>>                                              ^
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotJVMCICompilerFactory.java:73:
>> warning: unexported type referenced in exported API
>>     public int adjustCompilationLevel(HotSpotVMConfig config,
>> Class<?> declaringClass, String name, String signature, boolean
>> isOsr, int level) {
>>                                       ^
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
>> warning: unexported type referenced in exported API
>>     public void notifyInstall(HotSpotCodeCacheProvider
>> hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
>> compiledCode) {
>>                               ^
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
>> warning: unexported type referenced in exported API
>>     public void notifyInstall(HotSpotCodeCacheProvider
>> hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
>> compiledCode) {
>>
>>           ^
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
>> warning: unexported type referenced in exported API
>>     public void notifyInstall(HotSpotCodeCacheProvider
>> hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
>> compiledCode) {
>>
>>                                        ^
>> 1 error
>> 7 warnings
>>
>> -jdk/java.desktop:
>> .../jdk/src/java.desktop/share/classes/javax/swing/JRootPane.java:333: warning:
>> inaccessible type referenced in exported API
>>     protected DefaultAction defaultPressAction;
>>               ^
>> .../jdk/src/java.desktop/share/classes/javax/swing/JRootPane.java:344: warning:
>> inaccessible type referenced in exported API
>>     protected DefaultAction defaultReleaseAction;
>>               ^
>> error: warnings found and -Werror specified
>> .../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java:742:
>> warning: inaccessible type referenced in exported API
>>         protected MetalBumps bumps = new MetalBumps( 10, 10,
>>                   ^
>> .../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalFileChooserUI.java:926:
>> warning: inaccessible type referenced in exported API
>>     protected DirectoryComboBoxRenderer
>> createDirectoryComboBoxRenderer(JFileChooser fc) {
>>               ^
>> .../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalScrollBarUI.java:65:
>> warning: inaccessible type referenced in exported API
>>     protected MetalBumps bumps;
>>               ^
>> Note: Some input files use or override a deprecated API.
>> Note: Recompile with -Xlint:deprecation for details.
>> 1 error
>> 5 warnings
>>
>>
>> -jdk/java.naming:
>> .../jdk/src/java.naming/share/classes/javax/naming/CompoundName.java:156:
>> warning: inaccessible type referenced in exported API
>>     protected transient NameImpl impl;
>>                         ^
>> error: warnings found and -Werror specified
>> 1 error
>> 1 warning
>>
>>
>> -jdk/jdk.accessibility:
>> .../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/AWTEventMonitor.java:219:
>> warning: inaccessible type referenced in exported API
>>     static protected AWTEventsListener awtListener = new
>> AWTEventsListener();
>>                      ^
>> error: warnings found and -Werror specified
>> .../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/AccessibilityEventMonitor.java:66:
>> warning: inaccessible type referenced in exported API
>>     static protected final AccessibilityEventListener
>> accessibilityListener =
>>                            ^
>> .../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/SwingEventMonitor.java:74:
>> warning: inaccessible type referenced in exported API
>>     static protected final SwingEventListener swingListener = new
>> SwingEventListener();
>>                            ^
>> 1 error
>> 3 warnings
>>
>>
>> -jdk/jdk.jsobject:
>> .../jdk/src/jdk.jsobject/share/classes/netscape/javascript/JSObject.java:153:
>> warning: type from a module referenced in exported API that is not
>> requires public from this module
>>     public static JSObject getWindow(Applet applet) throws JSException {
>>                                      ^
>> error: warnings found and -Werror specified
>> 1 error
>> 1 warning
>>
>> -jdk/jdk.security.jgss:
>> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSContext.java:42:
>> warning: type that is invisible for some modules which read this
>> package referenced in exported API
>>     static class ExtendedGSSContextImpl extends GSSContextImpl
>>                                                 ^
>> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSContext.java:45:
>> warning: type that is invisible for some modules which read this
>> package referenced in exported API
>>         public ExtendedGSSContextImpl(GSSContextImpl old) {
>>                                       ^
>> error: warnings found and -Werror specified
>> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSCredential.java:38:
>> warning: type that is invisible for some modules which read this
>> package referenced in exported API
>>     static class ExtendedGSSCredentialImpl extends GSSCredentialImpl
>>                                                    ^
>> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSCredential.java:41:
>> warning: type that is invisible for some modules which read this
>> package referenced in exported API
>>         public ExtendedGSSCredentialImpl(GSSCredentialImpl old) {
>>                                          ^
>> 1 error
>> 4 warnings
>
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

Alan Bateman


On 13/06/2016 20:26, Philip Race wrote:
> PS .. also you probably should  just suppress lint on the jdk.jsobject
> module
>
> The change you propose to JSObject is going to cause a potential conflict
> with the ongoing review discussion about this here :-
>
> http://mail.openjdk.java.net/pipermail/awt-dev/2016-June/011472.html
Speaking of, shouldn't this have forRemoval=true? The reason that
jdk.jsobject doesn't `requires public java.desktop` is because we expect
getWindow(Applet) will eventually be removed and we don't want to trip
up those that might be relying on it to access APIs exported by
java.desktop.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

Phil Race
Alan,

See the comment here :
http://mail.openjdk.java.net/pipermail/awt-dev/2016-June/011478.html

Probably you should chime in directly on that discussion with such input ..

-phil.

On 6/13/16, 12:47 PM, Alan Bateman wrote:

>
>
> On 13/06/2016 20:26, Philip Race wrote:
>> PS .. also you probably should  just suppress lint on the
>> jdk.jsobject module
>>
>> The change you propose to JSObject is going to cause a potential
>> conflict
>> with the ongoing review discussion about this here :-
>>
>> http://mail.openjdk.java.net/pipermail/awt-dev/2016-June/011472.html
> Speaking of, shouldn't this have forRemoval=true? The reason that
> jdk.jsobject doesn't `requires public java.desktop` is because we
> expect getWindow(Applet) will eventually be removed and we don't want
> to trip up those that might be relying on it to access APIs exported
> by java.desktop.
>
> -Alan
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

Mandy Chung
I agree JSObject.getWindow(Applet) should have forRemoval=true, as I raised in awt-dev thread.  The confusion there was when the API marked forRemoval=true in JDK 9 and when it should really be removed.

Mandy

> On Jun 13, 2016, at 12:53 PM, Philip Race <[hidden email]> wrote:
>
> Alan,
>
> See the comment here : http://mail.openjdk.java.net/pipermail/awt-dev/2016-June/011478.html
>
> Probably you should chime in directly on that discussion with such input ..
>
> -phil.
>
> On 6/13/16, 12:47 PM, Alan Bateman wrote:
>>
>>
>> On 13/06/2016 20:26, Philip Race wrote:
>>> PS .. also you probably should  just suppress lint on the jdk.jsobject module
>>>
>>> The change you propose to JSObject is going to cause a potential conflict
>>> with the ongoing review discussion about this here :-
>>>
>>> http://mail.openjdk.java.net/pipermail/awt-dev/2016-June/011472.html
>> Speaking of, shouldn't this have forRemoval=true? The reason that jdk.jsobject doesn't `requires public java.desktop` is because we expect getWindow(Applet) will eventually be removed and we don't want to trip up those that might be relying on it to access APIs exported by java.desktop.
>>
>> -Alan

Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

Alan Bateman
In reply to this post by jan.lahoda
On 13/06/2016 17:12, Jan Lahoda wrote:

> Hello,
>
> There is:
> https://bugs.openjdk.java.net/browse/JDK-8153362
>
> which is about a new warning that should be produced by javac when
> exported API refers to types not exported/accessible to the API clients.
>
> I've put my current javac change here:
> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
Did you have a short list of names for the lint option before deciding
on "unexportedinapi"? If time has already been put into this and this is
the best of a bad bunch then ignore my mail. I bring it up because it
feels more like a "potentiallynotaccessible" or "notaccessible" or
"leaksnotaccessible". For the cases where we have ended up with
protected fields in public classes but the field type is package-private
then the field is never accessible. For the JSObject.getWindow case then
consumers will need to require java.desktop to use this method.

Related is the description:

javac.opt.Xlint.desc.unexportedinapi=\
     Warn about use of types not visible to clients in exported API

Shouldn't get say something about the type potentially not accessible
rather than visible?

-Alan

PS: You asked about the JVMCI classes in the hotspot repo. While this
might look strange then it is intentional. The "framework" uses the
reflective APIs to export the otherwise internal packages to the JVMCI
implementation when it is located and loaded.
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

jan.lahoda
In reply to this post by Phil Race
Hi Phil,

Thanks for the comments. I was preferring @SuppressWarnings over
-Xlint:-unexportedinapi because it allows to disable the check only on
selected elements (as opposed to disabling it for a whole module, where
a newly added API might inadvertently still cause a new warning); and
also because it would be slightly easier to phase the change in. But I
have no problem with using -Xlint:-unexportedinapi, I'll work on that.

Thanks,
     Jan

On 13.6.2016 19:59, Phil Race wrote:

> Hmm .. given that the majority of the jdk changes are in client -
> specifically
> swing & accessibility -  including the swing mailing list would have
> increased the
> likelihood of the right people clicking on this webrev link.
>
> IMO,  we should remove these unusable fields from the Swing API - where
> we can - and I think some we can, and really it ought to be all.
>
> So I suggest that you leave alone those files and submit
> bugs against swing & accessibility and the area owner
> can then make the decision as to the appropriate treatment.
>
> You can keep the JDK buildable in the meanwhile by suppressing the
> lint warnings on the desktop module.
>
> -phil.
>
>
> On 06/13/2016 09:12 AM, Jan Lahoda wrote:
>> Hello,
>>
>> There is:
>> https://bugs.openjdk.java.net/browse/JDK-8153362
>>
>> which is about a new warning that should be produced by javac when
>> exported API refers to types not exported/accessible to the API clients.
>>
>> I've put my current javac change here:
>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
>>
>> There are some warnings produced for existing code in the hotspot and
>> jdk repositories (see the end for the warnings), I was using these
>> patches to keep JDK buildable:
>> hotspot repository:
>> http://cr.openjdk.java.net/~jlahoda/8153362/hotspot.00/
>> jdk repository:
>> http://cr.openjdk.java.net/~jlahoda/8153362/jdk.00/
>>
>> Any help with those (esp. those in the hotspot repository) would be
>> wholeheartedly welcome.
>>
>> Any feedback on this is welcome!
>>
>> Thanks,
>>    Jan
>>
>> The warnings:
>> -hotspot repository:
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.runtime/src/jdk/vm/ci/runtime/services/JVMCICompilerFactory.java:72:
>> warning: unexported type referenced in exported API
>>     public abstract JVMCICompiler createCompiler(JVMCIRuntime runtime);
>>                     ^
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.runtime/src/jdk/vm/ci/runtime/services/JVMCICompilerFactory.java:72:
>> warning: unexported type referenced in exported API
>>     public abstract JVMCICompiler createCompiler(JVMCIRuntime runtime);
>>                                                  ^
>> error: warnings found and -Werror specified
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotJVMCICompilerFactory.java:54:
>> warning: unexported type referenced in exported API
>>     public int getCompilationLevelAdjustment(HotSpotVMConfig config) {
>>                                              ^
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotJVMCICompilerFactory.java:73:
>> warning: unexported type referenced in exported API
>>     public int adjustCompilationLevel(HotSpotVMConfig config, Class<?>
>> declaringClass, String name, String signature, boolean isOsr, int
>> level) {
>>                                       ^
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
>> warning: unexported type referenced in exported API
>>     public void notifyInstall(HotSpotCodeCacheProvider
>> hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
>> compiledCode) {
>>                               ^
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
>> warning: unexported type referenced in exported API
>>     public void notifyInstall(HotSpotCodeCacheProvider
>> hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
>> compiledCode) {
>>
>>           ^
>> .../hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/services/HotSpotVMEventListener.java:70:
>> warning: unexported type referenced in exported API
>>     public void notifyInstall(HotSpotCodeCacheProvider
>> hotSpotCodeCacheProvider, InstalledCode installedCode, CompiledCode
>> compiledCode) {
>>
>>                                        ^
>> 1 error
>> 7 warnings
>>
>> -jdk/java.desktop:
>> .../jdk/src/java.desktop/share/classes/javax/swing/JRootPane.java:333:
>> warning: inaccessible type referenced in exported API
>>     protected DefaultAction defaultPressAction;
>>               ^
>> .../jdk/src/java.desktop/share/classes/javax/swing/JRootPane.java:344:
>> warning: inaccessible type referenced in exported API
>>     protected DefaultAction defaultReleaseAction;
>>               ^
>> error: warnings found and -Werror specified
>> .../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java:742:
>> warning: inaccessible type referenced in exported API
>>         protected MetalBumps bumps = new MetalBumps( 10, 10,
>>                   ^
>> .../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalFileChooserUI.java:926:
>> warning: inaccessible type referenced in exported API
>>     protected DirectoryComboBoxRenderer
>> createDirectoryComboBoxRenderer(JFileChooser fc) {
>>               ^
>> .../jdk/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalScrollBarUI.java:65:
>> warning: inaccessible type referenced in exported API
>>     protected MetalBumps bumps;
>>               ^
>> Note: Some input files use or override a deprecated API.
>> Note: Recompile with -Xlint:deprecation for details.
>> 1 error
>> 5 warnings
>>
>>
>> -jdk/java.naming:
>> .../jdk/src/java.naming/share/classes/javax/naming/CompoundName.java:156:
>> warning: inaccessible type referenced in exported API
>>     protected transient NameImpl impl;
>>                         ^
>> error: warnings found and -Werror specified
>> 1 error
>> 1 warning
>>
>>
>> -jdk/jdk.accessibility:
>> .../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/AWTEventMonitor.java:219:
>> warning: inaccessible type referenced in exported API
>>     static protected AWTEventsListener awtListener = new
>> AWTEventsListener();
>>                      ^
>> error: warnings found and -Werror specified
>> .../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/AccessibilityEventMonitor.java:66:
>> warning: inaccessible type referenced in exported API
>>     static protected final AccessibilityEventListener
>> accessibilityListener =
>>                            ^
>> .../jdk/src/jdk.accessibility/share/classes/com/sun/java/accessibility/util/SwingEventMonitor.java:74:
>> warning: inaccessible type referenced in exported API
>>     static protected final SwingEventListener swingListener = new
>> SwingEventListener();
>>                            ^
>> 1 error
>> 3 warnings
>>
>>
>> -jdk/jdk.jsobject:
>> .../jdk/src/jdk.jsobject/share/classes/netscape/javascript/JSObject.java:153:
>> warning: type from a module referenced in exported API that is not
>> requires public from this module
>>     public static JSObject getWindow(Applet applet) throws JSException {
>>                                      ^
>> error: warnings found and -Werror specified
>> 1 error
>> 1 warning
>>
>> -jdk/jdk.security.jgss:
>> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSContext.java:42:
>> warning: type that is invisible for some modules which read this
>> package referenced in exported API
>>     static class ExtendedGSSContextImpl extends GSSContextImpl
>>                                                 ^
>> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSContext.java:45:
>> warning: type that is invisible for some modules which read this
>> package referenced in exported API
>>         public ExtendedGSSContextImpl(GSSContextImpl old) {
>>                                       ^
>> error: warnings found and -Werror specified
>> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSCredential.java:38:
>> warning: type that is invisible for some modules which read this
>> package referenced in exported API
>>     static class ExtendedGSSCredentialImpl extends GSSCredentialImpl
>>                                                    ^
>> .../jdk/src/jdk.security.jgss/share/classes/com/sun/security/jgss/ExtendedGSSCredential.java:41:
>> warning: type that is invisible for some modules which read this
>> package referenced in exported API
>>         public ExtendedGSSCredentialImpl(GSSCredentialImpl old) {
>>                                          ^
>> 1 error
>> 4 warnings
>
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

jan.lahoda
In reply to this post by Alan Bateman
Hi Alan,

On 14.6.2016 12:57, Alan Bateman wrote:

> On 13/06/2016 17:12, Jan Lahoda wrote:
>
>> Hello,
>>
>> There is:
>> https://bugs.openjdk.java.net/browse/JDK-8153362
>>
>> which is about a new warning that should be produced by javac when
>> exported API refers to types not exported/accessible to the API clients.
>>
>> I've put my current javac change here:
>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
> Did you have a short list of names for the lint option before deciding
> on "unexportedinapi"? If time has already been put into this and this is

I had a few (e.g. "publishingunexported"), but none of them particularly
nice.

> the best of a bad bunch then ignore my mail. I bring it up because it
> feels more like a "potentiallynotaccessible" or "notaccessible" or
> "leaksnotaccessible". For the cases where we have ended up with

I like "leaksnotaccessible". Unless there would be better ideas or
objections, I'd go with that. Thanks for the ideas!

> protected fields in public classes but the field type is package-private
> then the field is never accessible. For the JSObject.getWindow case then
> consumers will need to require java.desktop to use this method.
>
> Related is the description:
>
> javac.opt.Xlint.desc.unexportedinapi=\
>      Warn about use of types not visible to clients in exported API
>
> Shouldn't get say something about the type potentially not accessible
> rather than visible?

Yes, it should. I'll fix that. Thanks for catching that.

Jan

>
> -Alan
>
> PS: You asked about the JVMCI classes in the hotspot repo. While this
> might look strange then it is intentional. The "framework" uses the
> reflective APIs to export the otherwise internal packages to the JVMCI
> implementation when it is located and loaded.
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

jan.lahoda
Hi,

I've updated the patches, reflecting the feedback so far.

The langtools change is now split into two parts, one is only adding the
new lint key (but no checks are actually performed):
http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase1/

And the second part is adding the checks:
http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase2/

We could push the first part first, and the second one together with
other changes later, so that the repositories don't have to be updated
in a lockstep.

In addition to the langtools changes, only the top-level repository
needs to be changed now, to disable the checks in some modules:
http://cr.openjdk.java.net/~jlahoda/8153362/top-level.01/

Any feedback is welcome!

Thanks,
     Jan

On 14.6.2016 14:29, Jan Lahoda wrote:

> Hi Alan,
>
> On 14.6.2016 12:57, Alan Bateman wrote:
>> On 13/06/2016 17:12, Jan Lahoda wrote:
>>
>>> Hello,
>>>
>>> There is:
>>> https://bugs.openjdk.java.net/browse/JDK-8153362
>>>
>>> which is about a new warning that should be produced by javac when
>>> exported API refers to types not exported/accessible to the API clients.
>>>
>>> I've put my current javac change here:
>>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
>> Did you have a short list of names for the lint option before deciding
>> on "unexportedinapi"? If time has already been put into this and this is
>
> I had a few (e.g. "publishingunexported"), but none of them particularly
> nice.
>
>> the best of a bad bunch then ignore my mail. I bring it up because it
>> feels more like a "potentiallynotaccessible" or "notaccessible" or
>> "leaksnotaccessible". For the cases where we have ended up with
>
> I like "leaksnotaccessible". Unless there would be better ideas or
> objections, I'd go with that. Thanks for the ideas!
>
>> protected fields in public classes but the field type is package-private
>> then the field is never accessible. For the JSObject.getWindow case then
>> consumers will need to require java.desktop to use this method.
>>
>> Related is the description:
>>
>> javac.opt.Xlint.desc.unexportedinapi=\
>>      Warn about use of types not visible to clients in exported API
>>
>> Shouldn't get say something about the type potentially not accessible
>> rather than visible?
>
> Yes, it should. I'll fix that. Thanks for catching that.
>
> Jan
>
>>
>> -Alan
>>
>> PS: You asked about the JVMCI classes in the hotspot repo. While this
>> might look strange then it is intentional. The "framework" uses the
>> reflective APIs to export the otherwise internal packages to the JVMCI
>> implementation when it is located and loaded.
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

Jonathan Gibbons
Since you have already admitted to using multiple concatenated lower
case words in unexportedinapi, and since "unexported" is not a real
word, can I suggest notexportedinapi for the category, and
NOT_EXPORTED_IN_API for the Lint category.   Or else just simply all the
was down to "exports" and "EXPORTS", by analogy with other language
features like "static", "cast", etc

Code changes look OK.  Long lines in final test are easy enough to read
in a new file (as here) but will be harder to read in futire
side-by-side webrevs.

If it helps there is a ModuleBuilder class in the toolbox package in the
langtools/test/lib directory.

-- Jon


In javac.properties, I think the text

  217     Warn about use of types not visible to clients in exported API

would read slightly better as

  217     Warn about use of types in exported API that are not visible to clients


Similarly , I think the wording of the messages in compiler.properties
could be improved somewhat:

For example, change

2846 compiler.warn.inaccessible.in.api=\
2847     inaccessible type referenced in exported API

to

      # 0: symbol kind, 1: symbol, 2:symbol
2846 compiler.warn.inaccessible.in.api=\
2847     {0} {1} in the exported API for module {2} is not accessible

but even that may not be right.  What exactly does "referenced in
exported API" mean? Can we get rid of the phrase altogether, as in


2846 compiler.warn.inaccessible.in.api=\

      # 0: symbol kind, 1: symbol, 2:symbol
2847{0} {1} in module {2} is not accessible
      # 0: symbol kind, 1: symbol, 2:symbol
2848 compiler.warn.unexported.in.api=\
2849{0} {1} in module {2} is not exported  
     # 0: symbol kind, 1: symbol, 2:symbol
2850 compiler.warn.unexported.in.api.not.required.public=\
2851{0} {1} in module {2}is not indirectly exported using 'requires public'  
     # 0: symbol kind, 1: symbol, 2:symbol
2852 compiler.warn.unexported.in.api.qualified=\
2853{0} {1} in module {2}  may not be visible to all clients that require this module

-- Jon

On 06/17/2016 07:18 AM, Jan Lahoda wrote:

> Hi,
>
> I've updated the patches, reflecting the feedback so far.
>
> The langtools change is now split into two parts, one is only adding
> the new lint key (but no checks are actually performed):
> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase1/
>
> And the second part is adding the checks:
> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase2/
>
> We could push the first part first, and the second one together with
> other changes later, so that the repositories don't have to be updated
> in a lockstep.
>
> In addition to the langtools changes, only the top-level repository
> needs to be changed now, to disable the checks in some modules:
> http://cr.openjdk.java.net/~jlahoda/8153362/top-level.01/
>
> Any feedback is welcome!
>
> Thanks,
>     Jan
>
> On 14.6.2016 14:29, Jan Lahoda wrote:
>> Hi Alan,
>>
>> On 14.6.2016 12:57, Alan Bateman wrote:
>>> On 13/06/2016 17:12, Jan Lahoda wrote:
>>>
>>>> Hello,
>>>>
>>>> There is:
>>>> https://bugs.openjdk.java.net/browse/JDK-8153362
>>>>
>>>> which is about a new warning that should be produced by javac when
>>>> exported API refers to types not exported/accessible to the API
>>>> clients.
>>>>
>>>> I've put my current javac change here:
>>>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
>>> Did you have a short list of names for the lint option before deciding
>>> on "unexportedinapi"? If time has already been put into this and
>>> this is
>>
>> I had a few (e.g. "publishingunexported"), but none of them particularly
>> nice.
>>
>>> the best of a bad bunch then ignore my mail. I bring it up because it
>>> feels more like a "potentiallynotaccessible" or "notaccessible" or
>>> "leaksnotaccessible". For the cases where we have ended up with
>>
>> I like "leaksnotaccessible". Unless there would be better ideas or
>> objections, I'd go with that. Thanks for the ideas!
>>
>>> protected fields in public classes but the field type is
>>> package-private
>>> then the field is never accessible. For the JSObject.getWindow case
>>> then
>>> consumers will need to require java.desktop to use this method.
>>>
>>> Related is the description:
>>>
>>> javac.opt.Xlint.desc.unexportedinapi=\
>>>      Warn about use of types not visible to clients in exported API
>>>
>>> Shouldn't get say something about the type potentially not accessible
>>> rather than visible?
>>
>> Yes, it should. I'll fix that. Thanks for catching that.
>>
>> Jan
>>
>>>
>>> -Alan
>>>
>>> PS: You asked about the JVMCI classes in the hotspot repo. While this
>>> might look strange then it is intentional. The "framework" uses the
>>> reflective APIs to export the otherwise internal packages to the JVMCI
>>> implementation when it is located and loaded.

Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

jan.lahoda
Thanks for the comments/suggestion, and apologies for a belated reply.

I've updated the patch to use -Xlint:exports, which I agree is more
consistent with the existing lint keys.

The updated patch for the first phase is here:
http://cr.openjdk.java.net/~jlahoda/8153362/langtools.02-phase1/

And for the second phase is here:
http://cr.openjdk.java.net/~jlahoda/8153362/langtools.02-phase2/

Updated patch for the top-level repository is here:
http://cr.openjdk.java.net/~jlahoda/8153362/top-level.02/

I've also filled JDK-8161376:
https://bugs.openjdk.java.net/browse/JDK-8161376

For the first phase.

Thanks!

Jan

On 27.6.2016 21:10, Jonathan Gibbons wrote:

> Since you have already admitted to using multiple concatenated lower
> case words in unexportedinapi, and since "unexported" is not a real
> word, can I suggest notexportedinapi for the category, and
> NOT_EXPORTED_IN_API for the Lint category.   Or else just simply all the
> was down to "exports" and "EXPORTS", by analogy with other language
> features like "static", "cast", etc
>
> Code changes look OK.  Long lines in final test are easy enough to read
> in a new file (as here) but will be harder to read in futire
> side-by-side webrevs.
>
> If it helps there is a ModuleBuilder class in the toolbox package in the
> langtools/test/lib directory.
>
> -- Jon
>
>
> In javac.properties, I think the text
>
>   217     Warn about use of types not visible to clients in exported API
>
> would read slightly better as
>
>   217     Warn about use of types in exported API that are not visible
> to clients
>
>
> Similarly , I think the wording of the messages in compiler.properties
> could be improved somewhat:
>
> For example, change
>
> 2846 compiler.warn.inaccessible.in.api=\
> 2847     inaccessible type referenced in exported API
>
> to
>
>       # 0: symbol kind, 1: symbol, 2:symbol
> 2846 compiler.warn.inaccessible.in.api=\
> 2847     {0} {1} in the exported API for module {2} is not accessible
>
> but even that may not be right.  What exactly does "referenced in
> exported API" mean? Can we get rid of the phrase altogether, as in
>
>
> 2846 compiler.warn.inaccessible.in.api=\
>
>       # 0: symbol kind, 1: symbol, 2:symbol
> 2847{0} {1} in module {2} is not accessible
>       # 0: symbol kind, 1: symbol, 2:symbol
> 2848 compiler.warn.unexported.in.api=\
> 2849{0} {1} in module {2} is not exported     # 0: symbol kind, 1:
> symbol, 2:symbol
> 2850 compiler.warn.unexported.in.api.not.required.public=\
> 2851{0} {1} in module {2}is not indirectly exported using 'requires
> public'     # 0: symbol kind, 1: symbol, 2:symbol
> 2852 compiler.warn.unexported.in.api.qualified=\
> 2853{0} {1} in module {2}  may not be visible to all clients that
> require this module
>
> -- Jon
>
> On 06/17/2016 07:18 AM, Jan Lahoda wrote:
>> Hi,
>>
>> I've updated the patches, reflecting the feedback so far.
>>
>> The langtools change is now split into two parts, one is only adding
>> the new lint key (but no checks are actually performed):
>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase1/
>>
>> And the second part is adding the checks:
>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase2/
>>
>> We could push the first part first, and the second one together with
>> other changes later, so that the repositories don't have to be updated
>> in a lockstep.
>>
>> In addition to the langtools changes, only the top-level repository
>> needs to be changed now, to disable the checks in some modules:
>> http://cr.openjdk.java.net/~jlahoda/8153362/top-level.01/
>>
>> Any feedback is welcome!
>>
>> Thanks,
>>     Jan
>>
>> On 14.6.2016 14:29, Jan Lahoda wrote:
>>> Hi Alan,
>>>
>>> On 14.6.2016 12:57, Alan Bateman wrote:
>>>> On 13/06/2016 17:12, Jan Lahoda wrote:
>>>>
>>>>> Hello,
>>>>>
>>>>> There is:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8153362
>>>>>
>>>>> which is about a new warning that should be produced by javac when
>>>>> exported API refers to types not exported/accessible to the API
>>>>> clients.
>>>>>
>>>>> I've put my current javac change here:
>>>>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
>>>> Did you have a short list of names for the lint option before deciding
>>>> on "unexportedinapi"? If time has already been put into this and
>>>> this is
>>>
>>> I had a few (e.g. "publishingunexported"), but none of them particularly
>>> nice.
>>>
>>>> the best of a bad bunch then ignore my mail. I bring it up because it
>>>> feels more like a "potentiallynotaccessible" or "notaccessible" or
>>>> "leaksnotaccessible". For the cases where we have ended up with
>>>
>>> I like "leaksnotaccessible". Unless there would be better ideas or
>>> objections, I'd go with that. Thanks for the ideas!
>>>
>>>> protected fields in public classes but the field type is
>>>> package-private
>>>> then the field is never accessible. For the JSObject.getWindow case
>>>> then
>>>> consumers will need to require java.desktop to use this method.
>>>>
>>>> Related is the description:
>>>>
>>>> javac.opt.Xlint.desc.unexportedinapi=\
>>>>      Warn about use of types not visible to clients in exported API
>>>>
>>>> Shouldn't get say something about the type potentially not accessible
>>>> rather than visible?
>>>
>>> Yes, it should. I'll fix that. Thanks for catching that.
>>>
>>> Jan
>>>
>>>>
>>>> -Alan
>>>>
>>>> PS: You asked about the JVMCI classes in the hotspot repo. While this
>>>> might look strange then it is intentional. The "framework" uses the
>>>> reflective APIs to export the otherwise internal packages to the JVMCI
>>>> implementation when it is located and loaded.
>
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

jan.lahoda
In reply to this post by jan.lahoda
Hello,

I've updated the patch to the current situation. The top-level
repository patch is here:
http://cr.openjdk.java.net/~jlahoda/8153362/top-level.03/

Langtools repository patch is here:
http://cr.openjdk.java.net/~jlahoda/8153362/langtools.04-phase2/

Does this look OK?

Thanks,
     Jan

On 17.6.2016 16:18, Jan Lahoda wrote:

> Hi,
>
> I've updated the patches, reflecting the feedback so far.
>
> The langtools change is now split into two parts, one is only adding the
> new lint key (but no checks are actually performed):
> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase1/
>
> And the second part is adding the checks:
> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase2/
>
> We could push the first part first, and the second one together with
> other changes later, so that the repositories don't have to be updated
> in a lockstep.
>
> In addition to the langtools changes, only the top-level repository
> needs to be changed now, to disable the checks in some modules:
> http://cr.openjdk.java.net/~jlahoda/8153362/top-level.01/
>
> Any feedback is welcome!
>
> Thanks,
>      Jan
>
> On 14.6.2016 14:29, Jan Lahoda wrote:
>> Hi Alan,
>>
>> On 14.6.2016 12:57, Alan Bateman wrote:
>>> On 13/06/2016 17:12, Jan Lahoda wrote:
>>>
>>>> Hello,
>>>>
>>>> There is:
>>>> https://bugs.openjdk.java.net/browse/JDK-8153362
>>>>
>>>> which is about a new warning that should be produced by javac when
>>>> exported API refers to types not exported/accessible to the API
>>>> clients.
>>>>
>>>> I've put my current javac change here:
>>>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
>>> Did you have a short list of names for the lint option before deciding
>>> on "unexportedinapi"? If time has already been put into this and this is
>>
>> I had a few (e.g. "publishingunexported"), but none of them particularly
>> nice.
>>
>>> the best of a bad bunch then ignore my mail. I bring it up because it
>>> feels more like a "potentiallynotaccessible" or "notaccessible" or
>>> "leaksnotaccessible". For the cases where we have ended up with
>>
>> I like "leaksnotaccessible". Unless there would be better ideas or
>> objections, I'd go with that. Thanks for the ideas!
>>
>>> protected fields in public classes but the field type is package-private
>>> then the field is never accessible. For the JSObject.getWindow case then
>>> consumers will need to require java.desktop to use this method.
>>>
>>> Related is the description:
>>>
>>> javac.opt.Xlint.desc.unexportedinapi=\
>>>      Warn about use of types not visible to clients in exported API
>>>
>>> Shouldn't get say something about the type potentially not accessible
>>> rather than visible?
>>
>> Yes, it should. I'll fix that. Thanks for catching that.
>>
>> Jan
>>
>>>
>>> -Alan
>>>
>>> PS: You asked about the JVMCI classes in the hotspot repo. While this
>>> might look strange then it is intentional. The "framework" uses the
>>> reflective APIs to export the otherwise internal packages to the JVMCI
>>> implementation when it is located and loaded.
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

jan.lahoda
Hi,

Any feedback on the top-level repository changes?
http://cr.openjdk.java.net/~jlahoda/8153362/top-level.03/

Thanks!

Jan

On 13.9.2016 16:28, Jan Lahoda wrote:

> Hello,
>
> I've updated the patch to the current situation. The top-level
> repository patch is here:
> http://cr.openjdk.java.net/~jlahoda/8153362/top-level.03/
>
> Langtools repository patch is here:
> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.04-phase2/
>
> Does this look OK?
>
> Thanks,
>      Jan
>
> On 17.6.2016 16:18, Jan Lahoda wrote:
>> Hi,
>>
>> I've updated the patches, reflecting the feedback so far.
>>
>> The langtools change is now split into two parts, one is only adding the
>> new lint key (but no checks are actually performed):
>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase1/
>>
>> And the second part is adding the checks:
>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.01-phase2/
>>
>> We could push the first part first, and the second one together with
>> other changes later, so that the repositories don't have to be updated
>> in a lockstep.
>>
>> In addition to the langtools changes, only the top-level repository
>> needs to be changed now, to disable the checks in some modules:
>> http://cr.openjdk.java.net/~jlahoda/8153362/top-level.01/
>>
>> Any feedback is welcome!
>>
>> Thanks,
>>      Jan
>>
>> On 14.6.2016 14:29, Jan Lahoda wrote:
>>> Hi Alan,
>>>
>>> On 14.6.2016 12:57, Alan Bateman wrote:
>>>> On 13/06/2016 17:12, Jan Lahoda wrote:
>>>>
>>>>> Hello,
>>>>>
>>>>> There is:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8153362
>>>>>
>>>>> which is about a new warning that should be produced by javac when
>>>>> exported API refers to types not exported/accessible to the API
>>>>> clients.
>>>>>
>>>>> I've put my current javac change here:
>>>>> http://cr.openjdk.java.net/~jlahoda/8153362/langtools.00/
>>>> Did you have a short list of names for the lint option before deciding
>>>> on "unexportedinapi"? If time has already been put into this and
>>>> this is
>>>
>>> I had a few (e.g. "publishingunexported"), but none of them particularly
>>> nice.
>>>
>>>> the best of a bad bunch then ignore my mail. I bring it up because it
>>>> feels more like a "potentiallynotaccessible" or "notaccessible" or
>>>> "leaksnotaccessible". For the cases where we have ended up with
>>>
>>> I like "leaksnotaccessible". Unless there would be better ideas or
>>> objections, I'd go with that. Thanks for the ideas!
>>>
>>>> protected fields in public classes but the field type is
>>>> package-private
>>>> then the field is never accessible. For the JSObject.getWindow case
>>>> then
>>>> consumers will need to require java.desktop to use this method.
>>>>
>>>> Related is the description:
>>>>
>>>> javac.opt.Xlint.desc.unexportedinapi=\
>>>>      Warn about use of types not visible to clients in exported API
>>>>
>>>> Shouldn't get say something about the type potentially not accessible
>>>> rather than visible?
>>>
>>> Yes, it should. I'll fix that. Thanks for catching that.
>>>
>>> Jan
>>>
>>>>
>>>> -Alan
>>>>
>>>> PS: You asked about the JVMCI classes in the hotspot repo. While this
>>>> might look strange then it is intentional. The "framework" uses the
>>>> reflective APIs to export the otherwise internal packages to the JVMCI
>>>> implementation when it is located and loaded.
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

Alan Bateman
On 27/09/2016 13:42, Jan Lahoda wrote:

> Hi,
>
> Any feedback on the top-level repository changes?
> http://cr.openjdk.java.net/~jlahoda/8153362/top-level.03/
This looks okay to me. Some of these should be looked at and I wonder if
you have created any bugs.

In passing I see recipes for jdk.dev_XXXX that should be removed as they
aren't used. Not your issue so we can create another issue to tracking
removing them (assuming you don't want to touch this).

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

jan.lahoda
On 27.9.2016 14:49, Alan Bateman wrote:
> On 27/09/2016 13:42, Jan Lahoda wrote:
>
>> Hi,
>>
>> Any feedback on the top-level repository changes?
>> http://cr.openjdk.java.net/~jlahoda/8153362/top-level.03/
> This looks okay to me. Some of these should be looked at and I wonder if
> you have created any bugs.

My plan is to file one bug per module before push.

>
> In passing I see recipes for jdk.dev_XXXX that should be removed as they
> aren't used. Not your issue so we can create another issue to tracking
> removing them (assuming you don't want to touch this).

I could do that if needed, but I am not sure I understand all the
context of these recipes.

Thanks for the comments.

Jan

>
> -Alan
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

cowwoc
Administrator
In reply to this post by jan.lahoda
Hi Jan,

I've got a specific use-case I'd like your feedback on:

* My library exports an enum.
* Users must be able to pass enum values into exported API methods.
* The enum contains a method to ensure that each possible value defines a
different implementation.
* The method is only meant to be invoked by non-exported implementation
classes; however, seeing as those classes reside in other packages the
method is public.

So I'm left with a public method that I don't want users to be able to
invoke. Ideally I don't want them to see the method either, but I don't
think there is much I can do about that.

Does this represent a legitimate case for public, non-exported methods? Or
is there a better design that would meet the requirements?

Thank you,
Gili



--
Sent from: http://jigsaw-dev.1059479.n5.nabble.com/
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

Alan Bateman
On 12/10/2017 05:01, cowwoc wrote:

> Hi Jan,
>
> I've got a specific use-case I'd like your feedback on:
>
> * My library exports an enum.
> * Users must be able to pass enum values into exported API methods.
> * The enum contains a method to ensure that each possible value defines a
> different implementation.
> * The method is only meant to be invoked by non-exported implementation
> classes; however, seeing as those classes reside in other packages the
> method is public.
>
> So I'm left with a public method that I don't want users to be able to
> invoke. Ideally I don't want them to see the method either, but I don't
> think there is much I can do about that.
>
> Does this represent a legitimate case for public, non-exported methods? Or
> is there a better design that would meet the requirements?
Your library is a module that exports a package with a public enum. The
enum defines a method that is not intended to be directly used outside
of the module and so needs to be non-public.

One approach is to put a helper class in a non-exported package that
calls the non-public method on the enum reflectively (code in a module
can do deep reflection on any member of any class in its own module).
Using reflection, even on oneself, is brittle of course.

A type safe approach would be to define a helper interface in a
non-exported package and have the enum register an implementation.
Static methods on the interface, or a separate helper, would provide
consumers in the module a way to obtain the implementation. We uses this
approach in the java.base module where it known as the "shared secrets
mechanism".

As regards "seeing the method" then there is nothing to prevent nosy
parkers from reflecting on the enum class and discovering all its
declared methods, including non-public methods. They can't invoke the
methods but they will know they exist.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: JDK-8153362: [jigsaw] Add javac -Xlint warning to list exposed types which are not accessible

cowwoc
Administrator
Alan Bateman wrote
> A type safe approach would be to define a helper interface in a
> non-exported package and have the enum register an implementation.
> Static methods on the interface, or a separate helper, would provide
> consumers in the module a way to obtain the implementation. We uses this
> approach in the java.base module where it known as the "shared secrets
> mechanism".

Hi Alan,

Can you provide an example of the "shared secrets mechanism"? You mentioned
the java.base module but it's not clear where to look. It might also be
easier if you reply at https://stackoverflow.com/q/46701545/14731 so others
can see your answer.

Thanks,
Gili



--
Sent from: http://jigsaw-dev.1059479.n5.nabble.com/