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/webrev/
<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.html
<http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049603.html>
Paul.
