RFR 8187742 Minimal set of bootstrap methods for dynamic constants
Hi, Please review the patch that adds a minimal set of bootstrap methods can be be used for producing dynamic constants: http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webrev/> The aim is to provide a small but common set of BSMs that are likely useful with ldc or as BSM arguments, filling in the gaps for constants that cannot be currently represented, such as null, primitive classes and VarHandles. It’s possible to get more sophisticated regarding say factories but that is something to consider later on. This patch is based off the minimal dynamic constant support (still in review): http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.htm... <http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.html> Paul.
Hi Paul, You have an import static requireNonNull but it is only used once in nullConstant, all other methods use Objects.requireNonNull. The test that checks that lookup, name and type are null or not is different in each method, so by example in primitiveClass, if type equals null, you get an IAE. I fail to see the point of using validateClassAccess(), if the BSM is used through an indy, the type is created only if the caller class can create it, so the test seems redundant. If it's not used by an indy, why do we need to test that ? Also, why it's not called in invoke ? There is also no reason to validate that the parameter type is the right one because the VM will validate the return type of the BSM is asTypable to the type sent as argument. Some methods use generics, so other don't. Given it's not useful to use generics if the methods is used as a BSM, i think the generics signature should be removed, otherwise, getstatic and invoke should use generics. In nullConstant, you do not need a requireNonNull here, getting you call isPrimitive() just after. In primitiveClass, the second test is equivalent to name.length() != 1, to remove the @SuppressWarnings, type should be declared as a Class<Class<?>>. Why not using getstatic to retrieve the field Type of the wrapper instead ? If you have invoke(), you do not need enumConstant because you can cook a constant method handle Enum.valueOf and send it to invoke. The methods that returns VarHandles are trickier because you need a Lookup object. getstatic should be renamed to getConstant because this is what it does. wrapping the Throwable in a LinkageError seems wrong to me given that a calls to a BSM already does that, so getstatic can just declare "throws Throawble" like invoke and you have the same semantics. in arrayVarHandle, the doc said that lookup is not used but lookup is used. in validateClassAccess, the return can be moved at the end of the method. and as a minor issue, all the ifs should be followed by curly braces per the coding convention. cheers, Rémi ----- Mail original -----
De: "Paul Sandoz" <paul.sandoz@oracle.com> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>, "hotspot-dev" <hotspot-dev@openjdk.java.net> Envoyé: Mardi 7 Novembre 2017 19:38:57 Objet: RFR 8187742 Minimal set of bootstrap methods for dynamic constants
Hi,
Please review the patch that adds a minimal set of bootstrap methods can be be used for producing dynamic constants:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webrev/>
The aim is to provide a small but common set of BSMs that are likely useful with ldc or as BSM arguments, filling in the gaps for constants that cannot be currently represented, such as null, primitive classes and VarHandles. It’s possible to get more sophisticated regarding say factories but that is something to consider later on.
This patch is based off the minimal dynamic constant support (still in review):
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.htm... <http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.html>
Paul.
Hi Remi, You are asking a lot of the same questions we went through a number of times before landing where we are :-)
On 7 Nov 2017, at 11:33, Remi Forax <forax@univ-mlv.fr> wrote:
Hi Paul,
You have an import static requireNonNull but it is only used once in nullConstant, all other methods use Objects.requireNonNull.
Fixed.
The test that checks that lookup, name and type are null or not is different in each method, so by example in primitiveClass, if type equals null, you get an IAE.
Fixed.
I fail to see the point of using validateClassAccess(), if the BSM is used through an indy, the type is created only if the caller class can create it, so the test seems redundant. If it's not used by an indy, why do we need to test that ?
We are being deliberately conservative ensuring the lookup is consistent with the type in case nefarious byte code spinners punch a hole (not proven a hole can be punched, just being conservative).
Also, why it's not called in invoke ?
What “it” are you referring to?
There is also no reason to validate that the parameter type is the right one because the VM will validate the return type of the BSM is asTypable to the type sent as argument.
Yes, but we prefer that the BSM reject thereby better signalling the source of the error.
Some methods use generics, so other don't. Given it's not useful to use generics if the methods is used as a BSM, i think the generics signature should be removed, otherwise, getstatic and invoke should use generics.
I updated the nullConstant method and removed the type variable. In general we try to be consistent with the explicit erasure unless it causes some contortion in the implementation as is the case for Enum.
In nullConstant, you do not need a requireNonNull here, getting you call isPrimitive() just after.
We decided to be explicit rather than implicit for all null checks.
In primitiveClass, the second test is equivalent to name.length() != 1, to remove the @SuppressWarnings, type should be declared as a Class<Class<?>>. Why not using getstatic to retrieve the field Type of the wrapper instead ?
Try passing Class.class to it. To be honest this is somewhat motivated by testing when called explicitly.
If you have invoke(), you do not need enumConstant because you can cook a constant method handle Enum.valueOf and send it to invoke.
It’s also possible to support via getStatic as well, as is the case for primitive class. We went back and forth on the generic and specific axis. For cases where we considered constants are “honorary" members of the constant pool we provide explicit BSMs.
The methods that returns VarHandles are trickier because you need a Lookup object.
getstatic should be renamed to getConstant because this is what it does.
No, we want to stick closely with the notion of what the BSM does, there is a close connection also with MethodHandle getStatic, it’s performing a static field access. “getConstant” is too vague, notionally all the BSMs return constants.
wrapping the Throwable in a LinkageError seems wrong to me given that a calls to a BSM already does that, so getstatic can just declare "throws Throawble" like invoke and you have the same semantics.
We don’t want to declare Throwable for the API in this case. This try/catch is just ceremony since a getstatic MH in the implementation can only throw a VM error e.g. related to out of memory or stack overflow. I can add some comments in the code. Note that reflective exceptions are mapped to their equivalent errors. Although the BSM has been linked it is being asked to perform what can be considered further linkage.
in arrayVarHandle, the doc said that lookup is not used but lookup is used.
Fixed.
in validateClassAccess, the return can be moved at the end of the method.
and as a minor issue, all the ifs should be followed by curly braces per the coding convention.
I expanded to curly braces. Paul.
----- Mail original -----
De: "Paul Sandoz" <paul.sandoz@oracle.com> À: "Remi Forax" <forax@univ-mlv.fr> Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net>, "hotspot-dev" <hotspot-dev@openjdk.java.net> Envoyé: Mardi 7 Novembre 2017 21:54:06 Objet: Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants
Hi Remi,
You are asking a lot of the same questions we went through a number of times before landing where we are :-)
On 7 Nov 2017, at 11:33, Remi Forax <forax@univ-mlv.fr> wrote:
Hi Paul,
You have an import static requireNonNull but it is only used once in nullConstant, all other methods use Objects.requireNonNull.
Fixed.
The test that checks that lookup, name and type are null or not is different in each method, so by example in primitiveClass, if type equals null, you get an IAE.
Fixed.
I fail to see the point of using validateClassAccess(), if the BSM is used through an indy, the type is created only if the caller class can create it, so the test seems redundant. If it's not used by an indy, why do we need to test that ?
We are being deliberately conservative ensuring the lookup is consistent with the type in case nefarious byte code spinners punch a hole (not proven a hole can be punched, just being conservative).
Also, why it's not called in invoke ?
What “it” are you referring to?
validateClassAccess.
There is also no reason to validate that the parameter type is the right one because the VM will validate the return type of the BSM is asTypable to the type sent as argument.
Yes, but we prefer that the BSM reject thereby better signalling the source of the error.
the method returns a Class in its signature, so it can not be something else if not call by a condy and if the type is wrong in the bytecode, the VM will throw a CCE with the same info than your IAE. i still think it's unnecessary.
Some methods use generics, so other don't. Given it's not useful to use generics if the methods is used as a BSM, i think the generics signature should be removed, otherwise, getstatic and invoke should use generics.
I updated the nullConstant method and removed the type variable.
In general we try to be consistent with the explicit erasure unless it causes some contortion in the implementation as is the case for Enum.
In nullConstant, you do not need a requireNonNull here, getting you call isPrimitive() just after.
We decided to be explicit rather than implicit for all null checks.
In primitiveClass, the second test is equivalent to name.length() != 1, to remove the @SuppressWarnings, type should be declared as a Class<Class<?>>. Why not using getstatic to retrieve the field Type of the wrapper instead ?
Try passing Class.class to it. To be honest this is somewhat motivated by testing when called explicitly.
see my answer to John.
If you have invoke(), you do not need enumConstant because you can cook a constant method handle Enum.valueOf and send it to invoke.
It’s also possible to support via getStatic as well, as is the case for primitive class.
We went back and forth on the generic and specific axis. For cases where we considered constants are “honorary" members of the constant pool we provide explicit BSMs.
The methods that returns VarHandles are trickier because you need a Lookup object.
getstatic should be renamed to getConstant because this is what it does.
No, we want to stick closely with the notion of what the BSM does, there is a close connection also with MethodHandle getStatic, it’s performing a static field access. “getConstant” is too vague, notionally all the BSMs return constants.
wrapping the Throwable in a LinkageError seems wrong to me given that a calls to a BSM already does that, so getstatic can just declare "throws Throawble" like invoke and you have the same semantics.
We don’t want to declare Throwable for the API in this case. This try/catch is just ceremony since a getstatic MH in the implementation can only throw a VM error e.g. related to out of memory or stack overflow. I can add some comments in the code.
but you do that in invoke().
Note that reflective exceptions are mapped to their equivalent errors. Although the BSM has been linked it is being asked to perform what can be considered further linkage.
in arrayVarHandle, the doc said that lookup is not used but lookup is used.
Fixed.
in validateClassAccess, the return can be moved at the end of the method.
and as a minor issue, all the ifs should be followed by curly braces per the coding convention.
I expanded to curly braces.
Paul.
Rémi
On Nov 7, 2017, at 2:18 PM, forax@univ-mlv.fr wrote:
Also, why it's not called in invoke ?
What “it” are you referring to?
validateClassAccess.
Because having a MH means you have already been granted access to invoke it. Having a Class does *not* mean you have been granted access to its members; that is a further step. — John
De: "John Rose" <john.r.rose@oracle.com> À: "Rémi Forax" <forax@univ-mlv.fr> Cc: "Paul Sandoz" <paul.sandoz@oracle.com>, "hotspot-dev" <hotspot-dev@openjdk.java.net>, "core-libs-dev" <core-libs-dev@openjdk.java.net> Envoyé: Mardi 7 Novembre 2017 23:20:01 Objet: Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants
On Nov 7, 2017, at 2:18 PM, [ mailto:forax@univ-mlv.fr | forax@univ-mlv.fr ] wrote:
Also, why it's not called in invoke ?
What “it” are you referring to?
validateClassAccess.
Because having a MH means you have already been granted access to invoke it. Having a Class does *not* mean you have been granted access to its members; that is a further step.
but here you are using the class to change the mh, so you can have access to the mh and still have no access to the Class if you do not call the BSM with a condy, which is the purpose of validateClassAccess, no ?
— John
Rémi
On 7 Nov 2017, at 14:18, forax@univ-mlv.fr wrote:
Try passing Class.class to it. To be honest this is somewhat motivated by testing when called explicitly.
see my answer to John.
Fair point, i’ll go with Class<?>. It’s the less smelly option. Updated along with other changes: http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webrev/>
If you have invoke(), you do not need enumConstant because you can cook a constant method handle Enum.valueOf and send it to invoke.
It’s also possible to support via getStatic as well, as is the case for primitive class.
We went back and forth on the generic and specific axis. For cases where we considered constants are “honorary" members of the constant pool we provide explicit BSMs.
The methods that returns VarHandles are trickier because you need a Lookup object.
getstatic should be renamed to getConstant because this is what it does.
No, we want to stick closely with the notion of what the BSM does, there is a close connection also with MethodHandle getStatic, it’s performing a static field access. “getConstant” is too vague, notionally all the BSMs return constants.
wrapping the Throwable in a LinkageError seems wrong to me given that a calls to a BSM already does that, so getstatic can just declare "throws Throawble" like invoke and you have the same semantics.
We don’t want to declare Throwable for the API in this case. This try/catch is just ceremony since a getstatic MH in the implementation can only throw a VM error e.g. related to out of memory or stack overflow. I can add some comments in the code.
but you do that in invoke().
There is no choice when Invoking a method handle that is passed to us. In the case of getStatic the use of a MH can be considered an implementation detail (we could use reflection) and we know what the j.l.invoke implementation does. In this case i believe only VM errors can be produced. Paul.
Good comments! I can handle a couple of them… On Nov 7, 2017, at 11:33 AM, Remi Forax <forax@univ-mlv.fr> wrote:
I fail to see the point of using validateClassAccess(), if the BSM is used through an indy, the type is created only if the caller class can create it, so the test seems redundant.
That's true, but…
If it's not used by an indy, why do we need to test that ? Also, why it's not called in invoke ?
…Enum.valueOf doesn't do a security check; that is its choice. This means that if you pass it an enum type that is not public or not in a package exported to you, you can still peek at its enum values. Meanwhile, when javac emits a reference to an enum, it does so with getstatic. The getstatic bytecode *does* perform access checks. The call to validateClassAccess performs those checks, for alignment with the semantics of getstatic. The internal use of Enum.valueOf is just a detail of the emulation of getstatic in the case of an enum. (Note to self: Never use enums to implement a shared secrets pattern.) For bootstrap methods I prefer to use the most restrictive set of applicable access rules, handshaking with the lookup. In the case of enums it doesn't matter much, as you say, because Enum.valueOf leaves the door open. I could go either way on this one.
There is also no reason to validate that the parameter type is the right one because the VM will validate the return type of the BSM is asTypable to the type sent as argument.
For condy, having the BSM validate types is a code smell for the reason you mention. Also, when primitives (and value types) are in the mix, people usually code the validation incorrectly, since Class.isInstance is the wrong tool, and the right tool is non-obvious (MHs.identity.asType). Are you commenting on the return type adjustment in invoke? That's just a minor optimization to avoid (un)boxing, which should have no semantic effect.
Some methods use generics, so other don't. Given it's not useful to use generics if the methods is used as a BSM, i think the generics signature should be removed, otherwise, getstatic and invoke should use generics.
The generics are present to make normal calls from source code type check. Use cases: Combinators, test code.
to remove the @SuppressWarnings, type should be declared as a Class<Class<?>>.
Paul explained this one to me when I made the same objection. Turns out that the class literal "Class.class" (which is required to call that BSM from source code) can only have a raw type. Yuck. So we declare the formal with a matching amount of raw-ness.
Why not using getstatic to retrieve the field Type of the wrapper instead ?
Because the descriptor is a more compact notation for a primitive type. Primitive type references will be common and we want them to be small.
If you have invoke(), you do not need enumConstant because you can cook a constant method handle Enum.valueOf and send it to invoke. The methods that returns VarHandles are trickier because you need a Lookup object.
The set is minimal, but not that minimal. We *could* express *everything* using invoke but that would be grotesquely verbose and opaque. The "extra" API points are thought to be helpful in reducing class file size, and also clarifying the intentions of the affected constants.
getstatic should be renamed to getConstant because this is what it does.
(I could go either way; since the JVM has ConstantValue attrs, "Constant" is a Thing. If it's getstatic I prefer getStatic, for alignment with pre-existing names in jli.)
wrapping the Throwable in a LinkageError seems wrong to me given that a calls to a BSM already does that, so getstatic can just declare "throws Throawble" like invoke and you have the same semantics.
The point is that we are emulating the semantics of a getstatic instruction, which only throws LEs. We don't want other stuff to leak out of the implementation. Remember that "bytecode behavior" is a normative specification, to use when applicable. It's applicable here. Again, I could go either way on this one. Thanks, — John
----- Mail original -----
De: "John Rose" <john.r.rose@oracle.com> À: "Rémi Forax" <forax@univ-mlv.fr> Cc: "Paul Sandoz" <paul.sandoz@oracle.com>, "hotspot-dev" <hotspot-dev@openjdk.java.net>, "core-libs-dev" <core-libs-dev@openjdk.java.net> Envoyé: Mardi 7 Novembre 2017 21:59:41 Objet: Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants
Good comments! I can handle a couple of them…
On Nov 7, 2017, at 11:33 AM, Remi Forax <forax@univ-mlv.fr> wrote:
I fail to see the point of using validateClassAccess(), if the BSM is used through an indy, the type is created only if the caller class can create it, so the test seems redundant.
That's true, but…
If it's not used by an indy, why do we need to test that ? Also, why it's not called in invoke ?
…Enum.valueOf doesn't do a security check; that is its choice. This means that if you pass it an enum type that is not public or not in a package exported to you, you can still peek at its enum values. Meanwhile, when javac emits a reference to an enum, it does so with getstatic. The getstatic bytecode *does* perform access checks. The call to validateClassAccess performs those checks, for alignment with the semantics of getstatic. The internal use of Enum.valueOf is just a detail of the emulation of getstatic in the case of an enum.
(Note to self: Never use enums to implement a shared secrets pattern.)
For bootstrap methods I prefer to use the most restrictive set of applicable access rules, handshaking with the lookup.
In the case of enums it doesn't matter much, as you say, because Enum.valueOf leaves the door open.
I could go either way on this one.
As you said, javac use getstatic (the bytecode), why not reuse getstatic (the BSM) in that case, being a field of an enum doesn't seem important and at least you are sure that you have the semantics of getstatic (you miss the cache in java.lang.Class but i'm not sure it worth it).
There is also no reason to validate that the parameter type is the right one because the VM will validate the return type of the BSM is asTypable to the type sent as argument.
For condy, having the BSM validate types is a code smell for the reason you mention. Also, when primitives (and value types) are in the mix, people usually code the validation incorrectly, since Class.isInstance is the wrong tool, and the right tool is non-obvious (MHs.identity.asType).
Are you commenting on the return type adjustment in invoke? That's just a minor optimization to avoid (un)boxing, which should have no semantic effect.
no, the return type adjustment is ok for me, it avoid unboxing + boxing if the original handle returns a primitive.
Some methods use generics, so other don't. Given it's not useful to use generics if the methods is used as a BSM, i think the generics signature should be removed, otherwise, getstatic and invoke should use generics.
The generics are present to make normal calls from source code type check. Use cases: Combinators, test code.
so getstatic and invoke should have generics in their signature too.
to remove the @SuppressWarnings, type should be declared as a Class<Class<?>>.
Paul explained this one to me when I made the same objection. Turns out that the class literal "Class.class" (which is required to call that BSM from source code) can only have a raw type. Yuck. So we declare the formal with a matching amount of raw-ness.
yes, right X.class and x.getClass() are the two rules in the JLS that can introduce raw types. I think i prefer 'type' to be typed as a Class<?> in that case, it's not a raw type and you can assign Class<Class>> to it.
Why not using getstatic to retrieve the field Type of the wrapper instead ?
Because the descriptor is a more compact notation for a primitive type. Primitive type references will be common and we want them to be small.
Ok, you have to send the wrapper type if you use getstatic, so you are saving a reference to the wrapper type.
If you have invoke(), you do not need enumConstant because you can cook a constant method handle Enum.valueOf and send it to invoke. The methods that returns VarHandles are trickier because you need a Lookup object.
The set is minimal, but not that minimal. We *could* express *everything* using invoke but that would be grotesquely verbose and opaque. The "extra" API points are thought to be helpful in reducing class file size, and also clarifying the intentions of the affected constants.
getstatic should be renamed to getConstant because this is what it does.
(I could go either way; since the JVM has ConstantValue attrs, "Constant" is a Thing. If it's getstatic I prefer getStatic, for alignment with pre-existing names in jli.)
perhaps getFinalStatic, because it's restricted to final field.
wrapping the Throwable in a LinkageError seems wrong to me given that a calls to a BSM already does that, so getstatic can just declare "throws Throawble" like invoke and you have the same semantics.
The point is that we are emulating the semantics of a getstatic instruction, which only throws LEs. We don't want other stuff to leak out of the implementation. Remember that "bytecode behavior" is a normative specification, to use when applicable. It's applicable here.
it's not the catch line 155 that bother me, it's the other line 165 (and 162).
Again, I could go either way on this one.
Thanks, — John
Rémi
On Nov 7, 2017, at 1:35 PM, forax@univ-mlv.fr wrote:
perhaps getFinalStatic, because it's restricted to final field.
The restriction to final field is because we are expecting the method to be used from condy, where it seems to be a syntax error to sample a constant from a non-constant variable. That said, I too would prefer to make it explicit (even in the name) that the BSM applies only to static final fields. So +1 for getStaticFinal instead of getStatic. We also discussed this internally back and forth. Your comment pushed me over the edge. ("getFinalStatic" is wrong BTW because "Final" is an extra constraint after "getStatic", and because "static final" is the usual written order for the corresponding modifiers, not "final static", at least in the relevant source bases.) — John
On 8 Nov 2017, at 08:59, John Rose <john.r.rose@oracle.com> wrote:
On Nov 7, 2017, at 1:35 PM, forax@univ-mlv.fr <mailto:forax@univ-mlv.fr> wrote:
perhaps getFinalStatic, because it's restricted to final field.
The restriction to final field is because we are expecting the method to be used from condy, where it seems to be a syntax error to sample a constant from a non-constant variable.
That said, I too would prefer to make it explicit (even in the name) that the BSM applies only to static final fields.
So +1 for getStaticFinal instead of getStatic. We also discussed this internally back and forth. Your comment pushed me over the edge.
Updated the CSR to reflect this. I will update the webrev soon.
("getFinalStatic" is wrong BTW because "Final" is an extra constraint after "getStatic", and because "static final" is the usual written order for the corresponding modifiers, not "final static", at least in the relevant source bases.)
+1 Paul.
On 7 Nov 2017, at 12:59, John Rose <john.r.rose@oracle.com> wrote:
Good comments! I can handle a couple of them…
On Nov 7, 2017, at 11:33 AM, Remi Forax <forax@univ-mlv.fr> wrote:
I fail to see the point of using validateClassAccess(), if the BSM is used through an indy, the type is created only if the caller class can create it, so the test seems redundant.
That's true, but…
If it's not used by an indy, why do we need to test that ? Also, why it's not called in invoke ?
…Enum.valueOf doesn't do a security check; that is its choice. This means that if you pass it an enum type that is not public or not in a package exported to you, you can still peek at its enum values. Meanwhile, when javac emits a reference to an enum, it does so with getstatic. The getstatic bytecode *does* perform access checks. The call to validateClassAccess performs those checks, for alignment with the semantics of getstatic. The internal use of Enum.valueOf is just a detail of the emulation of getstatic in the case of an enum.
(Note to self: Never use enums to implement a shared secrets pattern.)
For bootstrap methods I prefer to use the most restrictive set of applicable access rules, handshaking with the lookup.
In the case of enums it doesn't matter much, as you say, because Enum.valueOf leaves the door open.
Yes, Brian and I noticed that so we punted on the access control.
I could go either way on this one.
For consistency with our conservative position on other BSMs we should probably perform the checks. I’ll update.
There is also no reason to validate that the parameter type is the right one because the VM will validate the return type of the BSM is asTypable to the type sent as argument.
For condy, having the BSM validate types is a code smell for the reason you mention.
If the BSM should only accept specific types it seems within its remit to reject thereby better associating the error with the dynamic constant entry. We are dealing here with known fixed types (Class and VarHandle).
Also, when primitives (and value types) are in the mix, people usually code the validation incorrectly, since Class.isInstance is the wrong tool, and the right tool is non-obvious (MHs.identity.asType).
Are you commenting on the return type adjustment in invoke? That's just a minor optimization to avoid (un)boxing, which should have no semantic effect.
Some methods use generics, so other don't. Given it's not useful to use generics if the methods is used as a BSM, i think the generics signature should be removed, otherwise, getstatic and invoke should use generics.
The generics are present to make normal calls from source code type check. Use cases: Combinators, test code.
to remove the @SuppressWarnings, type should be declared as a Class<Class<?>>.
Paul explained this one to me when I made the same objection. Turns out that the class literal "Class.class" (which is required to call that BSM from source code) can only have a raw type. Yuck. So we declare the formal with a matching amount of raw-ness.
Why not using getstatic to retrieve the field Type of the wrapper instead ?
Because the descriptor is a more compact notation for a primitive type. Primitive type references will be common and we want them to be small.
Yes, i forgot to mention the size aspect.
If you have invoke(), you do not need enumConstant because you can cook a constant method handle Enum.valueOf and send it to invoke. The methods that returns VarHandles are trickier because you need a Lookup object.
The set is minimal, but not that minimal. We *could* express *everything* using invoke but that would be grotesquely verbose and opaque. The "extra" API points are thought to be helpful in reducing class file size, and also clarifying the intentions of the affected constants.
Yes.
getstatic should be renamed to getConstant because this is what it does.
(I could go either way; since the JVM has ConstantValue attrs, "Constant" is a Thing. If it's getstatic I prefer getStatic, for alignment with pre-existing names in jli.)
+1 on getStatic alignment with jli. getConstant is too general for this case IMO. The naming convention we chose was to append Constant if the prefix was a keyword such as null or enum. Paul.
wrapping the Throwable in a LinkageError seems wrong to me given that a calls to a BSM already does that, so getstatic can just declare "throws Throawble" like invoke and you have the same semantics.
The point is that we are emulating the semantics of a getstatic instruction, which only throws LEs. We don't want other stuff to leak out of the implementation. Remember that "bytecode behavior" is a normative specification, to use when applicable. It's applicable here.
Again, I could go either way on this one.
Thanks, — John
On 7 Nov 2017, at 13:54, Paul Sandoz <paul.sandoz@oracle.com> wrote:
If it's not used by an indy, why do we need to test that ? Also, why it's not called in invoke ?
…Enum.valueOf doesn't do a security check; that is its choice. This means that if you pass it an enum type that is not public or not in a package exported to you, you can still peek at its enum values. Meanwhile, when javac emits a reference to an enum, it does so with getstatic. The getstatic bytecode *does* perform access checks. The call to validateClassAccess performs those checks, for alignment with the semantics of getstatic. The internal use of Enum.valueOf is just a detail of the emulation of getstatic in the case of an enum.
(Note to self: Never use enums to implement a shared secrets pattern.)
For bootstrap methods I prefer to use the most restrictive set of applicable access rules, handshaking with the lookup.
In the case of enums it doesn't matter much, as you say, because Enum.valueOf leaves the door open.
Yes, Brian and I noticed that so we punted on the access control.
Hold on… no we didn’t, we included the explicit access control check. Paul.
On Nov 7, 2017, at 12:59 PM, John Rose <john.r.rose@oracle.com> wrote:
For condy, having the BSM validate types is a code smell for the reason you mention. Also, when primitives (and value types) are in the mix, people usually code the validation incorrectly, since Class.isInstance is the wrong tool, and the right tool is non-obvious (MHs.identity.asType).
For the record, here is a method that performs the correct conversions: https://bugs.openjdk.java.net/browse/JDK-8190982 The bug suggests that it be called Class::convert. The code itself is simple enough to cut-n-paste into your own BSMs. — John
Hi Paul, How about changing the name of the class to ConstantFactory to align with its two siblings - LambdaMetafactory and StringConcatFactory? Other comments are purely about the spec text (should I be posting them in the CSR issue instead?) The class-level javadoc could use more text to describe the contents (e.g. "Methods to facilitate creation of common kinds of dynamic constants...", similar to other bootstrap factories) and motivation for having this class. I think giving the motivation here is important - it might be not obvious why, for example, nullConstant is needed when there is aconst_null. Maybe an example of an invokedynamic using static arguments that are dynamic constants (repeat the last six words together BTW!) could also be there (javap- or jasm-style pseudocode for that?) Also, @author and @since are missing. All methods repeat the same NPE condition @throws NullPointerException if any used argument is {@code null} which looks especially strange in methods with one used parameter (e.g. nullConstant) or with no unused parameters (e.g. getStaticFinal). I'd suggest to instead go with the commonly used shortcut of declaring the NPE in the class-level Javadoc, e.g. "Passing a null argument to a method in this class will cause of NullPointerException to be thrown, unless the argument is specified to be unused, or unless otherwise noted". The VarHandle methods have @param type unused; must be {@code Class<VarHandle>} Looks like the "unused;" part should be removed. Thanks, Stas On 08.11.2017 0:08, Paul Sandoz wrote:
Hi,
Please review the patch that adds a minimal set of bootstrap methods can be be used for producing dynamic constants:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webrev/>
The aim is to provide a small but common set of BSMs that are likely useful with ldc or as BSM arguments, filling in the gaps for constants that cannot be currently represented, such as null, primitive classes and VarHandles. It’s possible to get more sophisticated regarding say factories but that is something to consider later on.
This patch is based off the minimal dynamic constant support (still in review):
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.htm... <http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.html>
Paul.
I also noticed that Class::isPrimitive now says the following: "These objects may only be accessed via the following public static final variables, and are the only Class objects for which this method returns true." (the fields its talking about are the <Box>.TYPE) Should this text be tweaked? Thanks, Stas On 09.11.2017 13:54, stanislav lukyanov wrote:
Hi Paul,
How about changing the name of the class to ConstantFactory to align with its two siblings - LambdaMetafactory and StringConcatFactory?
Other comments are purely about the spec text (should I be posting them in the CSR issue instead?)
The class-level javadoc could use more text to describe the contents (e.g. "Methods to facilitate creation of common kinds of dynamic constants...", similar to other bootstrap factories) and motivation for having this class. I think giving the motivation here is important - it might be not obvious why, for example, nullConstant is needed when there is aconst_null. Maybe an example of an invokedynamic using static arguments that are dynamic constants (repeat the last six words together BTW!) could also be there (javap- or jasm-style pseudocode for that?) Also, @author and @since are missing.
All methods repeat the same NPE condition @throws NullPointerException if any used argument is {@code null} which looks especially strange in methods with one used parameter (e.g. nullConstant) or with no unused parameters (e.g. getStaticFinal). I'd suggest to instead go with the commonly used shortcut of declaring the NPE in the class-level Javadoc, e.g. "Passing a null argument to a method in this class will cause of NullPointerException to be thrown, unless the argument is specified to be unused, or unless otherwise noted".
The VarHandle methods have @param type unused; must be {@code Class<VarHandle>} Looks like the "unused;" part should be removed.
Thanks, Stas
On 08.11.2017 0:08, Paul Sandoz wrote:
Hi,
Please review the patch that adds a minimal set of bootstrap methods can be be used for producing dynamic constants:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webrev/>
The aim is to provide a small but common set of BSMs that are likely useful with ldc or as BSM arguments, filling in the gaps for constants that cannot be currently represented, such as null, primitive classes and VarHandles. It’s possible to get more sophisticated regarding say factories but that is something to consider later on.
This patch is based off the minimal dynamic constant support (still in review):
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.htm... <http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.html>
Paul.
Hi Paul, just some quick process-related questions. Is this intended to be targeted for jdk 10? Is the current implementation already available in a separate repository and/or branch? I've read in the RFR for 8186046 that some parts are currently being refined in the amber repository. Or is the complete implementation already available there? If I want to port this to ppc64, what's the best way to start and what do I need? Is it just the two webrevs for 8187742 and 8186046 on top of the jdk hs repo? Or is it a branch of the amber repo? You may know that there's currently a discussion going on about what is a JEP and what's the right way to implement and integrate a JEP into the mainline. The general idea is that only 'finished' JEPs will be targeted and integrated into the always feature complete main line. So is this and the review for 8186046 about the integration into the jdk repo (which shouldn't happen before the JEP is not targeted for jdk10) or is it just the review before the JEP can actually be targeted? This is important because there's not much time before the jdk10 repos will enter Rampdown phase 1 and we would still have to port this to ppc (and also ARM/SPARC) if it will be targeted for jdk 10. I don't want to question the merits and quality of the current implementation which I'm sure are great. For me as an external observer its just hard to oversee the current status of the implementation. Thanks, Volker On Thu, Nov 9, 2017 at 9:24 AM, stanislav lukyanov <stanislav.lukyanov@oracle.com> wrote:
Hi Paul,
How about changing the name of the class to ConstantFactory to align with its two siblings - LambdaMetafactory and StringConcatFactory?
Other comments are purely about the spec text (should I be posting them in the CSR issue instead?)
The class-level javadoc could use more text to describe the contents (e.g. "Methods to facilitate creation of common kinds of dynamic constants...", similar to other bootstrap factories) and motivation for having this class. I think giving the motivation here is important - it might be not obvious why, for example, nullConstant is needed when there is aconst_null. Maybe an example of an invokedynamic using static arguments that are dynamic constants (repeat the last six words together BTW!) could also be there (javap- or jasm-style pseudocode for that?) Also, @author and @since are missing.
All methods repeat the same NPE condition @throws NullPointerException if any used argument is {@code null} which looks especially strange in methods with one used parameter (e.g. nullConstant) or with no unused parameters (e.g. getStaticFinal). I'd suggest to instead go with the commonly used shortcut of declaring the NPE in the class-level Javadoc, e.g. "Passing a null argument to a method in this class will cause of NullPointerException to be thrown, unless the argument is specified to be unused, or unless otherwise noted".
The VarHandle methods have @param type unused; must be {@code Class<VarHandle>} Looks like the "unused;" part should be removed.
Thanks, Stas
On 08.11.2017 0:08, Paul Sandoz wrote:
Hi,
Please review the patch that adds a minimal set of bootstrap methods can be be used for producing dynamic constants:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webrev/>
The aim is to provide a small but common set of BSMs that are likely useful with ldc or as BSM arguments, filling in the gaps for constants that cannot be currently represented, such as null, primitive classes and VarHandles. It’s possible to get more sophisticated regarding say factories but that is something to consider later on.
This patch is based off the minimal dynamic constant support (still in review):
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.htm... <http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.html>
Paul.
Hi Volker,
On 9 Nov 2017, at 01:01, Volker Simonis <volker.simonis@gmail.com> wrote:
Hi Paul,
just some quick process-related questions.
Is this intended to be targeted for jdk 10?
Yes, that’s what we are aiming for. I am front loading reviews ahead of a propose to target (which should hopefully happen soon) to make better use of time given the shorter cycles, thus increasing the quality and giving heads up to other developers like yourself who are responsible for other platforms.
Is the current implementation already available in a separate repository and/or branch? I've read in the RFR for 8186046 that some parts are currently being refined in the amber repository. Or is the complete implementation already available there?
A super set of functionality is present in the amber repo under the condy branch. We pealed a sub-set of that off and created patches for the idk/hs repo tracked in: Minimal ConstantDynamic support https://bugs.openjdk.java.net/browse/JDK-8186046 Tool support for ConstantDynamic https://bugs.openjdk.java.net/browse/JDK-8186209 http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... Minimal set of bootstrap methods for dynamic constants https://bugs.openjdk.java.net/browse/JDK-8187742 http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre... While these are under review they might diverge a little from the condy branch in the amber repo but i will sync back up in batches.
If I want to port this to ppc64, what's the best way to start and what do I need? Is it just the two webrevs for 8187742 and 8186046 on top of the jdk hs repo? Or is it a branch of the amber repo?
Yes, and focus on the following applied to the jdk/hs repo: http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... There are tests in that patch that are currently targeted to run only on x86 that would otherwise tickle failures on other platforms.
You may know that there's currently a discussion going on about what is a JEP and what's the right way to implement and integrate a JEP into the mainline. The general idea is that only 'finished' JEPs will be targeted and integrated into the always feature complete main line. So is this and the review for 8186046 about the integration into the jdk repo (which shouldn't happen before the JEP is not targeted for jdk10) or is it just the review before the JEP can actually be targeted? This is important because there's not much time before the jdk10 repos will enter Rampdown phase 1 and we would still have to port this to ppc (and also ARM/SPARC) if it will be targeted for jdk 10.
I don't want to question the merits and quality of the current implementation which I'm sure are great. For me as an external observer its just hard to oversee the current status of the implementation.
I hear you, thanks for your patience. Is it a little clearer now? From my own perspective this is the first flight over the new release cycle territory, the ride might be a little bumpy while we learn to pilot this better :-) Paul.
On Thu, Nov 9, 2017 at 8:06 PM, Paul Sandoz <paul.sandoz@oracle.com> wrote:
Hi Volker,
On 9 Nov 2017, at 01:01, Volker Simonis <volker.simonis@gmail.com> wrote:
Hi Paul,
just some quick process-related questions.
Is this intended to be targeted for jdk 10?
Yes, that’s what we are aiming for. I am front loading reviews ahead of a propose to target (which should hopefully happen soon) to make better use of time given the shorter cycles, thus increasing the quality and giving heads up to other developers like yourself who are responsible for other platforms.
Is the current implementation already available in a separate repository and/or branch? I've read in the RFR for 8186046 that some parts are currently being refined in the amber repository. Or is the complete implementation already available there?
A super set of functionality is present in the amber repo under the condy branch. We pealed a sub-set of that off and created patches for the idk/hs repo tracked in:
Minimal ConstantDynamic support https://bugs.openjdk.java.net/browse/JDK-8186046 Tool support for ConstantDynamic https://bugs.openjdk.java.net/browse/JDK-8186209 http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-...
Minimal set of bootstrap methods for dynamic constants https://bugs.openjdk.java.net/browse/JDK-8187742 http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre...
While these are under review they might diverge a little from the condy branch in the amber repo but i will sync back up in batches.
If I want to port this to ppc64, what's the best way to start and what do I need? Is it just the two webrevs for 8187742 and 8186046 on top of the jdk hs repo? Or is it a branch of the amber repo?
Yes, and focus on the following applied to the jdk/hs repo:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-...
There are tests in that patch that are currently targeted to run only on x86 that would otherwise tickle failures on other platforms.
You may know that there's currently a discussion going on about what is a JEP and what's the right way to implement and integrate a JEP into the mainline. The general idea is that only 'finished' JEPs will be targeted and integrated into the always feature complete main line. So is this and the review for 8186046 about the integration into the jdk repo (which shouldn't happen before the JEP is not targeted for jdk10) or is it just the review before the JEP can actually be targeted? This is important because there's not much time before the jdk10 repos will enter Rampdown phase 1 and we would still have to port this to ppc (and also ARM/SPARC) if it will be targeted for jdk 10.
I don't want to question the merits and quality of the current implementation which I'm sure are great. For me as an external observer its just hard to oversee the current status of the implementation.
I hear you, thanks for your patience. Is it a little clearer now?
Thanks Paul! Things are much clearer now. I'll start looking at the changes required for ppc/s390. Regards, Volker
From my own perspective this is the first flight over the new release cycle territory, the ride might be a little bumpy while we learn to pilot this better :-)
Paul.
Hi,
On 9 Nov 2017, at 00:24, stanislav lukyanov <stanislav.lukyanov@oracle.com> wrote:
Hi Paul,
How about changing the name of the class to ConstantFactory to align with its two siblings - LambdaMetafactory and StringConcatFactory?
I would prefer to keep it as is. It is intended to cover many forms of BSMs producing dynamic constants. LMF and SCF are very focused and there is some precedent that they may be called explicitly as factories where as for dynamic constants there is less motivation for this.
Other comments are purely about the spec text (should I be posting them in the CSR issue instead?)
The class-level javadoc could use more text to describe the contents (e.g. "Methods to facilitate creation of common kinds of dynamic constants...", similar to other bootstrap factories) and motivation for having this class. I think giving the motivation here is important - it might be not obvious why, for example, nullConstant is needed when there is aconst_null. Maybe an example of an invokedynamic using static arguments that are dynamic constants (repeat the last six words together BTW!) could also be there (javap- or jasm-style pseudocode for that?)
I would prefer to follow up later on with more non-normative explanatory text. It will take some careful crafting and i don’t want that to side-track the review for the moment (including guidance on what forms of computed constants are acceptable).
Also, @author
We don’t add @author.
and @since are missing.
Added.
All methods repeat the same NPE condition @throws NullPointerException if any used argument is {@code null} which looks especially strange in methods with one used parameter (e.g. nullConstant) or with no unused parameters (e.g. getStaticFinal). I'd suggest to instead go with the commonly used shortcut of declaring the NPE in the class-level Javadoc, e.g. "Passing a null argument to a method in this class will cause of NullPointerException to be thrown, unless the argument is specified to be unused, or unless otherwise noted”.
I like that: * <p>The bootstrap methods in this class will throw a * {@code NullPointerException} for any reference argument that is {@code null}, * unless the argument is specified to be unused or specified to accept a * {@code null} value. And adjusted the invoke method: * @param args the arguments to pass to the method handle, as if with * {@link MethodHandle#invokeWithArguments}. Each argument may be * {@code null}. ... * @throws NullPointerException if {@code args} is {@code null} * (each argument of {@code args} may be {@code null}). We have been fleshing out the NPE behaviour because we know you will log bugs against us later on if we don’t :-)
The VarHandle methods have @param type unused; must be {@code Class<VarHandle>} Looks like the "unused;" part should be removed.
Updated to: * @param type the required result type (must be {@code Class<VarHandle>}) and added the null check.
On 9 Nov 2017, at 00:42, stanislav lukyanov <stanislav.lukyanov@oracle.com> wrote:
I also noticed that Class::isPrimitive now says the following: "These objects may only be accessed via the following public static final variables, and are the only Class objects for which this method returns true." (the fields its talking about are the <Box>.TYPE) Should this text be tweaked?
I think it would be a distraction for most developers. The BSM is doing nothing special in regard to what isPrimitive specifies it just dynamically returns the value of the associated static field. Paul.
On 10.11.2017 0:13, Paul Sandoz wrote:
<...> I would prefer to follow up later on with more non-normative explanatory text. It will take some careful crafting and i don’t want that to side-track the review for the moment (including guidance on what forms of computed constants are acceptable). Fair. Does it make sense to add a sub-task for this to JDK-8185992 to keep it on the radar? <...>
And adjusted the invoke method:
* @param args the arguments to pass to the method handle, as if with * {@link MethodHandle#invokeWithArguments}. Each argument may be * {@code null}. ... * @throws NullPointerException if {@code args} is {@code null} * (each argument of {@code args} may be {@code null}). I think the `@throws NPE` is not needed, at least not from the normative perspective. The class-level text about NPE doesn't assume anything about the contents of objects passed to the methods, and the second part of the `@param args` already clarifies the behavior for this particular case. Arguably having this `@throws` might improve clarity, but I would rather remove it.
We have been fleshing out the NPE behaviour because we know you will log bugs against us later on if we don’t :-) Yep, that's sounds familiar :)
<...> Thanks for the updates! Also, javac isn't going to use `::primitiveClass` instead of `getstatic Integer.TYPE` to load int.class, right? If it is, int.class probably needs to be changed to Integer.TYPE in sun.invoke.util.Wrapper to avoid cycles. Stas
On 10 Nov 2017, at 01:36, stanislav lukyanov <stanislav.lukyanov@oracle.com> wrote:
On 10.11.2017 0:13, Paul Sandoz wrote:
<...> I would prefer to follow up later on with more non-normative explanatory text. It will take some careful crafting and i don’t want that to side-track the review for the moment (including guidance on what forms of computed constants are acceptable). Fair. Does it make sense to add a sub-task for this to JDK-8185992 to keep it on the radar?
You would mind adding a sub-task with a tbd minor target?
<...>
And adjusted the invoke method:
* @param args the arguments to pass to the method handle, as if with * {@link MethodHandle#invokeWithArguments}. Each argument may be * {@code null}. ... * @throws NullPointerException if {@code args} is {@code null} * (each argument of {@code args} may be {@code null}). I think the `@throws NPE` is not needed, at least not from the normative perspective. The class-level text about NPE doesn't assume anything about the contents of objects passed to the methods, and the second part of the `@param args` already clarifies the behavior for this particular case. Arguably having this `@throws` might improve clarity, but I would rather remove it.
Ok, if you are fine with that i will remove it.
We have been fleshing out the NPE behaviour because we know you will log bugs against us later on if we don’t :-)
Yep, that's sounds familiar :)
<...>
Thanks for the updates!
Also, javac isn't going to use `::primitiveClass` instead of `getstatic Integer.TYPE` to load int.class, right? If it is, int.class probably needs to be changed to Integer.TYPE in sun.invoke.util.Wrapper to avoid cycles.
Yes, i noticed that too :-) There are no plans in this patch to update javac code generation for primitive classes but if we do we may need to modify the wrapper implementation as you point out. There are might other areas within the base module (specifically in j.l.invoke) where referring to say int.class may cause class initialization issues early in the startup. That’s awkward (just like we cannot use lambda expressions in certain cases), we might need to retain the old translation strategy just for code within the base module or support a compilation error. Paul.
On 15.11.2017 6:19, Paul Sandoz wrote:
On 10 Nov 2017, at 01:36, stanislav lukyanov <stanislav.lukyanov@oracle.com> wrote:
On 10.11.2017 0:13, Paul Sandoz wrote:
<...> I would prefer to follow up later on with more non-normative explanatory text. It will take some careful crafting and i don’t want that to side-track the review for the moment (including guidance on what forms of computed constants are acceptable). Fair. Does it make sense to add a sub-task for this to JDK-8185992 to keep it on the radar? You would mind adding a sub-task with a tbd minor target? TBD_Minor would mean an update release, right? I think it can't be a TBD_Minor then since it is technically a specification change and should happen in a Java SE release (maybe 11?), but maybe I'm wrong. Anyway, as long as that's logged and not completely forgotten, I'm happy :)
Thanks, Stas
<...>
And adjusted the invoke method:
* @param args the arguments to pass to the method handle, as if with * {@link MethodHandle#invokeWithArguments}. Each argument may be * {@code null}. ... * @throws NullPointerException if {@code args} is {@code null} * (each argument of {@code args} may be {@code null}). I think the `@throws NPE` is not needed, at least not from the normative perspective. The class-level text about NPE doesn't assume anything about the contents of objects passed to the methods, and the second part of the `@param args` already clarifies the behavior for this particular case. Arguably having this `@throws` might improve clarity, but I would rather remove it. Ok, if you are fine with that i will remove it.
We have been fleshing out the NPE behaviour because we know you will log bugs against us later on if we don’t :-) Yep, that's sounds familiar :)
<...>
Thanks for the updates!
Also, javac isn't going to use `::primitiveClass` instead of `getstatic Integer.TYPE` to load int.class, right? If it is, int.class probably needs to be changed to Integer.TYPE in sun.invoke.util.Wrapper to avoid cycles.
Yes, i noticed that too :-)
There are no plans in this patch to update javac code generation for primitive classes but if we do we may need to modify the wrapper implementation as you point out. There are might other areas within the base module (specifically in j.l.invoke) where referring to say int.class may cause class initialization issues early in the startup. That’s awkward (just like we cannot use lambda expressions in certain cases), we might need to retain the old translation strategy just for code within the base module or support a compilation error.
Paul.
On 14 Nov 2017, at 20:48, stanislav lukyanov <stanislav.lukyanov@oracle.com> wrote:
On 15.11.2017 6:19, Paul Sandoz wrote:
On 10 Nov 2017, at 01:36, stanislav lukyanov <stanislav.lukyanov@oracle.com> wrote:
On 10.11.2017 0:13, Paul Sandoz wrote:
<...> I would prefer to follow up later on with more non-normative explanatory text. It will take some careful crafting and i don’t want that to side-track the review for the moment (including guidance on what forms of computed constants are acceptable). Fair. Does it make sense to add a sub-task for this to JDK-8185992 to keep it on the radar? You would mind adding a sub-task with a tbd minor target? TBD_Minor would mean an update release, right? I think it can't be a TBD_Minor then since it is technically a specification change and should happen in a Java SE release (maybe 11?), but maybe I'm wrong.
I was thinking this issue would be just for non-normative text in an @apiNote. Whether that maps to tbd_x or not with the new release process i dunno. Let’s just use tbd_major for now. Paul.
Anyway, as long as that's logged and not completely forgotten, I'm happy :)
Thanks, Stas
participants (6)
-
forax＠univ-mlv.fr
-
John Rose
-
Paul Sandoz
-
Remi Forax
-
stanislav lukyanov
-
Volker Simonis