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 via email to