Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8246778: Compiler implementation for Sealed Classes (Second Preview) #1227

Conversation

@vicente-romero-oracle
Copy link
Contributor

@vicente-romero-oracle vicente-romero-oracle commented Nov 16, 2020

Please review the code for the second iteration of sealed classes. In this iteration we are:

  • Enhancing narrowing reference conversion to allow for stricter checking of cast conversions with respect to sealed type hierarchies
  • Also local classes are not considered when determining implicitly declared permitted direct subclasses of a sealed class or sealed interface
  • renaming Class::permittedSubclasses to Class::getPermittedSubclasses, still in the same method, the return type has been changed to Class<?>[] instead of the previous ClassDesc[]
  • adding code to make sure that annotations can't be sealed
  • improving some tests

TIA

Related specs:
Sealed Classes JSL
Sealed Classes JVMS
Additional: Contextual Keywords


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Testing

Linux x64 Linux x86 Windows x64 macOS x64
Build ✔️ (5/5 passed) ✔️ (2/2 passed) ✔️ (2/2 passed) ✔️ (2/2 passed)
Test (tier1) ✔️ (9/9 passed) ✔️ (9/9 passed) ✔️ (9/9 passed) ✔️ (9/9 passed)

Issue

  • JDK-8246778: Compiler implementation for Sealed Classes (Second Preview)

Contributors

  • Vicente Romero <vromero@openjdk.org>
  • Harold Seigel <hseigel@openjdk.org>

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1227/head:pull/1227
$ git checkout pull/1227

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Nov 16, 2020

👋 Welcome back vromero! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr label Nov 16, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Nov 16, 2020

@vicente-romero-oracle The following labels will be automatically applied to this pull request:

  • compiler
  • core-libs
  • hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 16, 2020

Webrevs

@vicente-romero-oracle
Copy link
Contributor Author

@vicente-romero-oracle vicente-romero-oracle commented Nov 16, 2020

/contributor add vromero,hsiegel

@openjdk
Copy link

@openjdk openjdk bot commented Nov 16, 2020

@vicente-romero-oracle Could not parse vromero,hsiegel as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

  • /contributor add @openjdk-bot
  • /contributor add duke
  • /contributor add J. Duke <duke@openjdk.org>
* @apiNote
* This method reports on a distinct concept of sealing from
* {@link Class#isSealed() Class::isSealed}.
*

This comment has been minimized.

@AlanBateman

AlanBateman Nov 16, 2020
Contributor

This API note will be very confusing to readers. I think the javadoc will need to be fleshed out and probably will need to link to a section the Package class description that defines the legacy concept of sealing.

This comment has been minimized.

@mlchung

mlchung Nov 17, 2020
Member

I agree. This @APinote needs more clarification to help the readers to understand the context here. One thing we could do in the Package class description to add a "Package Sealing" section that can also explain that it has no relationship to "sealed classes".

This comment has been minimized.

@mlchung

mlchung Nov 24, 2020
Member

I added an API note in Package::isSealed [1] to clarify sealed package vs sealed class or interface.

The API note you added in Class::isSealed can be clarified in a similar fashion like: "Sealed class or interface has no relationship with {@linkplain Package#isSealed package sealing}".

[1] 3c230b8

This comment has been minimized.

@lahodaj

lahodaj Nov 25, 2020
Contributor

Thanks for that update, Mandy! I've tweaked the API note as per your recommendation. I'll publish a fixed PR later, reflecting the other review comments as well.

@vicente-romero-oracle
Copy link
Contributor Author

@vicente-romero-oracle vicente-romero-oracle commented Nov 16, 2020

/contributor add vromero

@openjdk
Copy link

@openjdk openjdk bot commented Nov 16, 2020

@vicente-romero-oracle
Contributor Vicente Romero <vromero@openjdk.org> successfully added.

@vicente-romero-oracle
Copy link
Contributor Author

@vicente-romero-oracle vicente-romero-oracle commented Nov 16, 2020

/contributor add hsiegel

@openjdk
Copy link

@openjdk openjdk bot commented Nov 16, 2020

@vicente-romero-oracle Could not parse hsiegel as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

  • /contributor add @openjdk-bot
  • /contributor add duke
  • /contributor add J. Duke <duke@openjdk.org>
@vicente-romero-oracle
Copy link
Contributor Author

@vicente-romero-oracle vicente-romero-oracle commented Nov 16, 2020

/contributor add hseigel

@openjdk
Copy link

@openjdk openjdk bot commented Nov 16, 2020

@vicente-romero-oracle
Contributor Harold Seigel <hseigel@openjdk.org> successfully added.

Copy link
Contributor

@mcimadamore mcimadamore left a comment

I left some comments re. the changes in cast conversion routine

}
if ((t.tsym.isSealed() || s.tsym.isSealed())) {

This comment has been minimized.

@mcimadamore

mcimadamore Nov 16, 2020
Contributor

It would probably be better to only run this extra check if result == true, to minimize compatibility impact.

return false;
}
// if both are classes or both are interfaces, shortcut
if (ts.isInterface() == ss.isInterface()) {

This comment has been minimized.

@mcimadamore

mcimadamore Nov 16, 2020
Contributor

What happens with code like this?

interface A permits B { }
non-sealed interface B extends A { }
interface C { }

class D implements C, B { } // this is a valid witness for both A and C, but A and C are unrelated with subtyping

class Test {
  void m(A a, C c) {
     a = (A)c;
  }
}```
Note that, w/o sealed types, the above snippet compiles ok - e.g. casting C to A is not going to give problems (as there could be a common subtype D <: A, C).
} catch (IllegalArgumentException iae) {
throw new InternalError("Invalid type in permitted subclasses information: " + subclassName, iae);
}
public Class<?>[] getPermittedSubclasses() {

This comment has been minimized.

@mlchung

mlchung Nov 17, 2020
Member

What happens if a permitted subclass is not found? I see that getPermittedSubclasses0 ignores the entry if the class is not found. Should that be specified?

Have you considered whether security package access is needed (now that this method returns Class objects the caller may not have access to)? This needs to be discussed with the security team. If someone gets a hold of a sealed class (e.g. obj.getClass()), this method could leak other Class objects.

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 18, 2020

Mailing list message from David Holmes on compiler-dev:

Hi Vincente,

On 16/11/2020 11:36 pm, Vicente Romero wrote:

Please review the code for the second iteration of sealed classes. In this iteration we are:

- Enhancing narrowing reference conversion to allow for stricter checking of cast conversions with respect to sealed type hierarchies.
- Also local classes are not considered when determining implicitly declared permitted direct subclasses of a sealed class or sealed interface

The major change here seems to be that getPermittedSubclasses() now
returns actual Class objects instead of ClassDesc. My recollection from
earlier discussions here was that the use of ClassDesc was very
deliberate as the permitted subclasses may not actually exist and there
may be security concerns with returning them!

Cheers,
David
-----

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 23, 2020

Mailing list message from Harold Seigel on core-libs-dev:

Hi David,

Thanks for looking at this.

The intent was for method Class.permittedSubclasses() to be implemented
similarly to Class.getNestMembers().? Are you suggesting that a security
manager check be added to permittedSubclasses() similar to the security
manager check in getNestMembers()?

Thanks, Harold

On 11/18/2020 12:31 AM, David Holmes wrote:

Hi Vincente,

On 16/11/2020 11:36 pm, Vicente Romero wrote:

Please review the code for the second iteration of sealed classes. In
this iteration we are:

- Enhancing narrowing reference conversion to allow for stricter
checking of cast conversions with respect to sealed type hierarchies.
- Also local classes are not considered when determining implicitly
declared permitted direct subclasses of a sealed class or sealed
interface

The major change here seems to be that getPermittedSubclasses() now
returns actual Class objects instead of ClassDesc. My recollection
from earlier discussions here was that the use of ClassDesc was very
deliberate as the permitted subclasses may not actually exist and
there may be security concerns with returning them!

Cheers,
David
-----

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 24, 2020

Mailing list message from David Holmes on core-libs-dev:

Hi Harold,

On 24/11/2020 6:27 am, Harold Seigel wrote:

Hi David,

Thanks for looking at this.

The intent was for method Class.permittedSubclasses() to be implemented
similarly to Class.getNestMembers().? Are you suggesting that a security
manager check be added to permittedSubclasses() similar to the security
manager check in getNestMembers()?

No I'm suggesting the change to the API is plain wrong. :) Please see
discussion in the CSR.

Cheers,
David

Thanks, Harold

On 11/18/2020 12:31 AM, David Holmes wrote:

Hi Vincente,

On 16/11/2020 11:36 pm, Vicente Romero wrote:

Please review the code for the second iteration of sealed classes. In
this iteration we are:

- Enhancing narrowing reference conversion to allow for stricter
checking of cast conversions with respect to sealed type hierarchies.
- Also local classes are not considered when determining implicitly
declared permitted direct subclasses of a sealed class or sealed
interface

The major change here seems to be that getPermittedSubclasses() now
returns actual Class objects instead of ClassDesc. My recollection
from earlier discussions here was that the use of ClassDesc was very
deliberate as the permitted subclasses may not actually exist and
there may be security concerns with returning them!

Cheers,
David
-----

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 24, 2020

Mailing list message from Remi Forax on hotspot-dev:

----- Mail original -----

De: "David Holmes" <david.holmes at oracle.com>
?: "Harold David Seigel" <harold.seigel at oracle.com>, "Vicente Romero" <vromero at openjdk.java.net>, "compiler-dev"
<compiler-dev at openjdk.java.net>, "core-libs-dev" <core-libs-dev at openjdk.java.net>, "hotspot-dev"
<hotspot-dev at openjdk.java.net>
Envoy?: Mardi 24 Novembre 2020 02:04:55
Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second Preview)

Hi Harold,

On 24/11/2020 6:27 am, Harold Seigel wrote:

Hi David,

Thanks for looking at this.

The intent was for method Class.permittedSubclasses() to be implemented
similarly to Class.getNestMembers().? Are you suggesting that a security
manager check be added to permittedSubclasses() similar to the security
manager check in getNestMembers()?

No I'm suggesting the change to the API is plain wrong. :) Please see
discussion in the CSR.

Given that the CSR is closed, i will answer here.
There are two issues with the previous implementation of permittedSubclasses, first it's the only method that using method desc which means that people has to be aware on another non trivial concept (object that describes constant pool constant) to understand how to use the method then i've tested this API with my students, all but one what able to correctly derives the Class from a ClassDesc, so instead of asking every users of permittedSubclasses to go through the oops of getting Class from a ClassDesc, i think we can agree that it's better to move the burden from the user to the JDK implementors.

Cheers,
David

regards,
R?mi

Thanks, Harold

On 11/18/2020 12:31 AM, David Holmes wrote:

Hi Vincente,

On 16/11/2020 11:36 pm, Vicente Romero wrote:

Please review the code for the second iteration of sealed classes. In
this iteration we are:

- Enhancing narrowing reference conversion to allow for stricter
checking of cast conversions with respect to sealed type hierarchies.
- Also local classes are not considered when determining implicitly
declared permitted direct subclasses of a sealed class or sealed
interface

The major change here seems to be that getPermittedSubclasses() now
returns actual Class objects instead of ClassDesc. My recollection
from earlier discussions here was that the use of ClassDesc was very
deliberate as the permitted subclasses may not actually exist and
there may be security concerns with returning them!

Cheers,
David
-----

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 24, 2020

Mailing list message from David Holmes on core-libs-dev:

Hi Remi,

On 24/11/2020 7:45 pm, Remi Forax wrote:

----- Mail original -----

De: "David Holmes" <david.holmes at oracle.com>
?: "Harold David Seigel" <harold.seigel at oracle.com>, "Vicente Romero" <vromero at openjdk.java.net>, "compiler-dev"
<compiler-dev at openjdk.java.net>, "core-libs-dev" <core-libs-dev at openjdk.java.net>, "hotspot-dev"
<hotspot-dev at openjdk.java.net>
Envoy?: Mardi 24 Novembre 2020 02:04:55
Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second Preview)

Hi Harold,

On 24/11/2020 6:27 am, Harold Seigel wrote:

Hi David,

Thanks for looking at this.

The intent was for method Class.permittedSubclasses() to be implemented
similarly to Class.getNestMembers().? Are you suggesting that a security
manager check be added to permittedSubclasses() similar to the security
manager check in getNestMembers()?

No I'm suggesting the change to the API is plain wrong. :) Please see
discussion in the CSR.

Given that the CSR is closed, i will answer here.
There are two issues with the previous implementation of permittedSubclasses, first it's the only method that using method desc which means that people has to be aware on another non trivial concept (object that describes constant pool constant) to understand how to use the method then i've tested this API with my students, all but one what able to correctly derives the Class from a ClassDesc, so instead of asking every users of permittedSubclasses to go through the oops of getting Class from a ClassDesc, i think we can agree that it's better to move the burden from the user to the JDK implementors.

Why is the objective to get the Class objects? What purpose does that
serve? The original API provided a descriptor for the contents of the
permittedSubclasses attribute. I find it totally bizarre to have an API
whose role is now to attempt to load all the subclasses of a sealed class.

YMMV.

David

Cheers,
David

regards,
R?mi

Thanks, Harold

On 11/18/2020 12:31 AM, David Holmes wrote:

Hi Vincente,

On 16/11/2020 11:36 pm, Vicente Romero wrote:

Please review the code for the second iteration of sealed classes. In
this iteration we are:

- Enhancing narrowing reference conversion to allow for stricter
checking of cast conversions with respect to sealed type hierarchies.
- Also local classes are not considered when determining implicitly
declared permitted direct subclasses of a sealed class or sealed
interface

The major change here seems to be that getPermittedSubclasses() now
returns actual Class objects instead of ClassDesc. My recollection
from earlier discussions here was that the use of ClassDesc was very
deliberate as the permitted subclasses may not actually exist and
there may be security concerns with returning them!

Cheers,
David
-----

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 24, 2020

Mailing list message from forax at univ-mlv.fr on core-libs-dev:

----- Mail original -----

De: "David Holmes" <david.holmes at oracle.com>
?: "Remi Forax" <forax at univ-mlv.fr>
Cc: "Harold David Seigel" <harold.seigel at oracle.com>, "Vicente Romero" <vromero at openjdk.java.net>, "compiler-dev"
<compiler-dev at openjdk.java.net>, "core-libs-dev" <core-libs-dev at openjdk.java.net>, "hotspot-dev"
<hotspot-dev at openjdk.java.net>
Envoy?: Mardi 24 Novembre 2020 13:16:39
Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second Preview)

Hi Remi,

On 24/11/2020 7:45 pm, Remi Forax wrote:

----- Mail original -----

De: "David Holmes" <david.holmes at oracle.com>
?: "Harold David Seigel" <harold.seigel at oracle.com>, "Vicente Romero"
<vromero at openjdk.java.net>, "compiler-dev"
<compiler-dev at openjdk.java.net>, "core-libs-dev"
<core-libs-dev at openjdk.java.net>, "hotspot-dev"
<hotspot-dev at openjdk.java.net>
Envoy?: Mardi 24 Novembre 2020 02:04:55
Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second
Preview)

Hi Harold,

On 24/11/2020 6:27 am, Harold Seigel wrote:

Hi David,

Thanks for looking at this.

The intent was for method Class.permittedSubclasses() to be implemented
similarly to Class.getNestMembers().? Are you suggesting that a security
manager check be added to permittedSubclasses() similar to the security
manager check in getNestMembers()?

No I'm suggesting the change to the API is plain wrong. :) Please see
discussion in the CSR.

Given that the CSR is closed, i will answer here.
There are two issues with the previous implementation of permittedSubclasses,
first it's the only method that using method desc which means that people has
to be aware on another non trivial concept (object that describes constant pool
constant) to understand how to use the method then i've tested this API with my
students, all but one what able to correctly derives the Class from a
ClassDesc, so instead of asking every users of permittedSubclasses to go
through the oops of getting Class from a ClassDesc, i think we can agree that
it's better to move the burden from the user to the JDK implementors.

Why is the objective to get the Class objects? What purpose does that
serve?

The whole idea of the reflection is to provide the runtime view of Java the language.
Even if such thing does not exist.

The original API provided a descriptor for the contents of the
permittedSubclasses attribute. I find it totally bizarre to have an API
whose role is now to attempt to load all the subclasses of a sealed class.

It's as bizarre as Class.getClasses() loading all the member classes.

You are discovering that the reflection API is bizarre, and it is :)
It's not the view of the JVM, it's the view of Java at runtime, whatever it means.

YMMV.

David

R?mi

[1] https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Class.html#getClasses()

Cheers,
David

regards,
R?mi

Thanks, Harold

On 11/18/2020 12:31 AM, David Holmes wrote:

Hi Vincente,

On 16/11/2020 11:36 pm, Vicente Romero wrote:

Please review the code for the second iteration of sealed classes. In
this iteration we are:

- Enhancing narrowing reference conversion to allow for stricter
checking of cast conversions with respect to sealed type hierarchies.
- Also local classes are not considered when determining implicitly
declared permitted direct subclasses of a sealed class or sealed
interface

The major change here seems to be that getPermittedSubclasses() now
returns actual Class objects instead of ClassDesc. My recollection
from earlier discussions here was that the use of ClassDesc was very
deliberate as the permitted subclasses may not actually exist and
there may be security concerns with returning them!

Cheers,
David
-----

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 24, 2020

Mailing list message from David Holmes on core-libs-dev:

On 24/11/2020 11:56 pm, forax at univ-mlv.fr wrote:

----- Mail original -----

De: "David Holmes" <david.holmes at oracle.com>
?: "Remi Forax" <forax at univ-mlv.fr>
Cc: "Harold David Seigel" <harold.seigel at oracle.com>, "Vicente Romero" <vromero at openjdk.java.net>, "compiler-dev"
<compiler-dev at openjdk.java.net>, "core-libs-dev" <core-libs-dev at openjdk.java.net>, "hotspot-dev"
<hotspot-dev at openjdk.java.net>
Envoy?: Mardi 24 Novembre 2020 13:16:39
Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second Preview)

Hi Remi,

On 24/11/2020 7:45 pm, Remi Forax wrote:

----- Mail original -----

De: "David Holmes" <david.holmes at oracle.com>
?: "Harold David Seigel" <harold.seigel at oracle.com>, "Vicente Romero"
<vromero at openjdk.java.net>, "compiler-dev"
<compiler-dev at openjdk.java.net>, "core-libs-dev"
<core-libs-dev at openjdk.java.net>, "hotspot-dev"
<hotspot-dev at openjdk.java.net>
Envoy?: Mardi 24 Novembre 2020 02:04:55
Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second
Preview)

Hi Harold,

On 24/11/2020 6:27 am, Harold Seigel wrote:

Hi David,

Thanks for looking at this.

The intent was for method Class.permittedSubclasses() to be implemented
similarly to Class.getNestMembers().? Are you suggesting that a security
manager check be added to permittedSubclasses() similar to the security
manager check in getNestMembers()?

No I'm suggesting the change to the API is plain wrong. :) Please see
discussion in the CSR.

Given that the CSR is closed, i will answer here.
There are two issues with the previous implementation of permittedSubclasses,
first it's the only method that using method desc which means that people has
to be aware on another non trivial concept (object that describes constant pool
constant) to understand how to use the method then i've tested this API with my
students, all but one what able to correctly derives the Class from a
ClassDesc, so instead of asking every users of permittedSubclasses to go
through the oops of getting Class from a ClassDesc, i think we can agree that
it's better to move the burden from the user to the JDK implementors.

Why is the objective to get the Class objects? What purpose does that
serve?

The whole idea of the reflection is to provide the runtime view of Java the language.
Even if such thing does not exist.

And providing some kind of descriptor for an attribute fulfills that
role. Nothing says it has to produce Class objects.

The original API provided a descriptor for the contents of the
permittedSubclasses attribute. I find it totally bizarre to have an API
whose role is now to attempt to load all the subclasses of a sealed class.

It's as bizarre as Class.getClasses() loading all the member classes.

Not at all. Nested types are considered to be part of the same
implementation as the outer type. They are intimately related and all
part of the same accessibility domain.

Loading subtypes that likely exist outside of the current package (else
why would you need to be using sealed types) is a completely different
matter.

You are discovering that the reflection API is bizarre, and it is :)

I don't find it that bizarre.

It's not the view of the JVM, it's the view of Java at runtime, whatever it means.

It means whatever we implement it to mean. So if we implement it in
bizarre ways then it will be perceived as bizarre.

David
-----

YMMV.

David

R?mi

[1] https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Class.html#getClasses()

Cheers,
David

regards,
R?mi

Thanks, Harold

On 11/18/2020 12:31 AM, David Holmes wrote:

Hi Vincente,

On 16/11/2020 11:36 pm, Vicente Romero wrote:

Please review the code for the second iteration of sealed classes. In
this iteration we are:

- Enhancing narrowing reference conversion to allow for stricter
checking of cast conversions with respect to sealed type hierarchies.
- Also local classes are not considered when determining implicitly
declared permitted direct subclasses of a sealed class or sealed
interface

The major change here seems to be that getPermittedSubclasses() now
returns actual Class objects instead of ClassDesc. My recollection
from earlier discussions here was that the use of ClassDesc was very
deliberate as the permitted subclasses may not actually exist and
there may be security concerns with returning them!

Cheers,
David
-----

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 24, 2020

Mailing list message from Remi Forax on compiler-dev:

----- Mail original -----

De: "Mandy Chung" <mchung at openjdk.java.net>
?: "compiler-dev" <compiler-dev at openjdk.java.net>, "core-libs-dev" <core-libs-dev at openjdk.java.net>, "hotspot-dev"
<hotspot-dev at openjdk.java.net>
Envoy?: Mercredi 25 Novembre 2020 00:02:53
Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second Preview)

On Tue, 17 Nov 2020 00:25:51 GMT, Mandy Chung <mchung at openjdk.org> wrote:

src/java.base/share/classes/java/lang/Package.java line 227:

225: * This method reports on a distinct concept of sealing from
226: * {@link Class#isSealed() Class::isSealed}.
227: *

This API note will be very confusing to readers. I think the javadoc will need
to be fleshed out and probably will need to link to a section the Package class
description that defines the legacy concept of sealing.

I agree. This @APinote needs more clarification to help the readers to
understand the context here. One thing we could do in the Package class
description to add a "Package Sealing" section that can also explain that it
has no relationship to "sealed classes".

I added an API note in `Package::isSealed` [1] to clarify sealed package vs
sealed class or interface.

The API note you added in `Class::isSealed` can be clarified in a similar
fashion like: "Sealed class or interface has no relationship with {@linkplain
Package#isSealed package sealing}".

Hi Mandy,
given that almost nobody knows about sealed packages, i'm not sure that adding a reference to Package::isSealed in Class::isSealed actually helps, it might be confusing.

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 24, 2020

Mailing list message from Mandy Chung on core-libs-dev:

On 11/24/20 3:18 PM, Remi Forax wrote:

Hi Mandy,
given that almost nobody knows about sealed packages, i'm not sure that adding a reference to Package::isSealed in Class::isSealed actually helps, it might be confusing.

It's even better if the API note in `Class::isSealed` from PR is removed:
???? * This method reports on a distinct concept of sealing from
???? * {@link Package#isSealed() Package::isSealed}.

I'm good with that.?? Alan and I saw the proposed API note and just feel
it adds more confusion than helping (My comment is meant to be on
Class::isSealed but now I realized the comment is on Package::isSealed.?
sorry for the confusion.)

Mandy

@vicente-romero-oracle vicente-romero-oracle deleted the vicente-romero-oracle:JDK-8246778 branch Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment