@Supported design issues

[Joe Darcy]

On 02/28/2013 11:31 AM, mark.reinhold at oracle.com wrote:
> 2013/2/23 5:10 -0800, joe.darcy at oracle.com:
>> On 2/22/2013 3:04 PM, mark.reinhold at oracle.com wrote:
>>> ...
>>>
>>> - The annotation isn't a simple marker annotation, which is what I
>>> expected at first glance; it takes a boolean parameter.  Does this
>>> mean that we have to go add "@Supported(false)" to everything that's
>>> not supported?  I'd have thought that anything not marked
>>> "@Supported(true)" would by implication, well, not be supported.
>>> Does it mean that if I mark a package "@Supported(true)" I can use
>>> "@Supported(false)" on some of its member types?
>> Having Supported take a boolean value both allows the explicit statement
>> that an item is not supported as well as providing a possible transition
>> path from Supported(true) to Supported(false) to removed.
> Okay.  In that scenario what's the role of the existing @Deprecated
> annotation?

@Deprecated generally applies to all clients of an API. I don't think 
people tend to associate potential removal of an API with deprecation 
because we haven't (yet) removed any deprecated Java SE APIs from the 
platform since to date we have placed a higher value on preserving  
binary compatibility.

The deprecated-in-JDK-7 and JDK-specific apt API was removed in JDK 8, 
but that is the only removal of a deprecated API from the JDK I'm aware 
of offhand.

The jdk.Supported annotation is mostly a statement to users outside of 
the JDK itself.

>> We already have types in the JDK whose comments explicitly say "this is
>> not part of any supported API" (much of javac).
>> If there is no explicit opt-in to mark supportedness as well as
>> non-supportedness in my estimation that means the status of all the
>> unadorned APIs is uncertain: "Perhaps this interesting API was just
>> overlooked in being marked supported, I'll go ahead and us it anyway..."
> Okay, so that will give us a three-valued system:
>
>    - @Supported(true) -- supported
>    - @Supported(false) -- not supported
>    - No @Supported annotation -- unknown (but probably not supported)
>
> I'm still wondering whether marking a package "@Supported(true)" means
> that I can use "@Supported(false)" on some of its member types.  That
> would be convenient for cases such as the JMX OSMBeanFactory class that
> Alan mentioned.

If a package has a mixture of supported and non-supported types, I would 
say it should either *not* have a @jdk.Supported annotation itself, or 
if the types in the package were predominately one value or another, 
then the package annotation should match the prevailing value of the types.

Since types have a more concrete existence then packages, I regard the 
jdk.Supported information on package-info files to have a higher mixture 
of informative versus normative sentiment compared to the annotation on 
types.

[snip]

>
>>> - I agree with Martin that "supportedness", in the abstract, isn't a
>>> binary thing.  If we're going to define an annotation for broad use
>>> then we should at least consider a metric with more than two values.
>>> ...
>> The status quo today and for the last 15 years has been often sloppy
>> management of the types in com.sun.* Some of them are
>> supported/stable/official/whatever others are not. Which are which is
>> not clear. The closest mechanism to documenting this, aside what
>> whatever comments might be in the code and the few subsets with
>> published javadoc, are whether or not the types ends up in ct.sym
>> proto-module system and if it does, whether or not a warning is issued
>> when using the type.
>>
>> The ct.sym file is constructed by passing information from the docs make
>> target to a program living in the langtools repo. So today the mechanism
>> we have is a very an obscure system that does a poor job of conveying
>> this kind of information and is easy to circumvent.
> What we have today is certainly a maintenance headache for JDK
> developers, who have to understand the obscure makefiles involved in
> the construction of ct.sym.
>
> Between compile-time warnings and controlling javadoc output, however,
> I'd say that it does an okay job of conveying the "supportedness" of
> JDK-specific APIs to the rest of the world, though it could be better.
>
> What I don't understand is how doing all this with an annotation would
> be any harder to circumvent than what we have today.  Are you proposing
> to do something stronger than issue a compiler warning when people try
> to use an unsupported API?

The ct.sym mechanism we have today is compile-time only and the 
mechanism and all its warnings can be circumvented by adding a single 
option to javac; the option is described on stackoverflow, amongst other 
places. Therefore, it is fairly easy for someone to claim "but I didn't 
know" in regards to the status of a JDK-specific API.

Since any jdk.Supported annotations applied to types are more localized 
and more specific ("*this* type is or is not supported / stable / etc.") 
it is both easier for JDK developers to made incremental changes to the 
JDK code base and is it also easier for users of those types to see what 
is going on since any inspection of the types can reveal the annotation 
value.

>
>> If we go from that obscure system to an explicit boolean-valued
>> annotation, that is in my estimation a vast improvement both in clarity
>> and usability.
> I agree that it's an improvement, in that it makes it easier for tools
> beyond javac to determine the "supportedness" of an API.  I can well
> imagine IDEs leveraging this annotation to give advice to developers
> ahead of compile time.
>
> Do you plan to change the makefiles for ct.sym, and the non-SE javadoc,
> so that it's based on the new annotation rather than today's obscure
> {,NON_}CORE_PKGS.gmk files?  Otherwise the maintenance headache will
> just get worse.

My main concern for @Supported was actually accurately capturing the 
classification work Alan and others have already done as part of the 
pre-modularization effort. However, I agree it would be preferable to 
change how ct.sym was generated.

>
>>> These are, more or less, the Solaris "Stable", "Evolving",
>>> "Unstable", and "Internal" levels, which suggests a single
>>> "@Stability" annotation and an enum parameter with the values
>>> STABLE, EVOLVING, UNSTABLE, and INTERNAL.
>> As I indicated earlier in this thread, I agree there are more subtle
>> distinctions that can be of interest, but at times the better is the
>> enemy of the good and the first approximation of is this type or package
>> supported or not is a huge improvement of what we have today even if it
>> doesn't cover all the possible gradations.
> The better can be the enemy of the good, yet the expedient can be the
> enemy of the future.
>
> If we're going to define a new annotation with this much visibility then
> we should at least take the time to inventory the JDK-specific APIs that
> we have, and those we reasonably expect to have in the near future, to
> understand how many distinct levels are useful.

Agreed, and as indicated above, capturing the inventory that has already 
been done was the impetus for adding jdk.Supported at this time (I've 
thought about adding such a type to the JDK for several years).

>
> Would it make sense, e.g., for the streams SPI in Lambda to be marked
> "unstable" rather than "not supported", so that javadoc for it is
> generated yet no commitment is made to its current form?

No; I think it is preferable to keep the streams types as a JDK 
implementation artifact to allow full de facto flexibility in designing 
the future SPI in that area.

>
> Even if we think we only need two explicit levels today, a design that
> admits expansion is preferable to one that forever limits us to just two
> values.  An annotation that takes an enum, to which we can add values
> over time, would be more future-proof.

Technically, it would be possible to evolve a boolean-valued annotation 
to one that included a non-boolean value as well by adding a new method 
that had a default to the annotation type. For example

// Version 1
@interfaced Supported {
     boolean value() default true;
}

// Version 2
@interface Supported {
     boolean value() default true;
     Stability stability() default STABLE;
}

However, if what we eventually want to capture is "stability level" 
rather than supported-ness than having a single stability value from the 
start would of course be preferable.

That said, it terms of the exercise of going over the inventory of 
existing types, I think it can be helpful to at first be constrained to 
making a binary supported / not-supported determination to avoid the 
temptation to overuse a middle-of-the-road value like EVOLVING.

Types that don't fit well into supported / not-supported classification 
can help drive what other distinctions are useful to make.


>
>>> - The retention policy of the annotation is RUNTIME.  Is that really
>>> what we want?  I'd have expected CLASS.
>> CLASS is not very helpful (and certainly not a helpful default). A
>> CLASS-retention annotation can be reliably used at the compile-time of
>> other code. For the use case of Supported, I think it is more helpful to
>> allow runtime querying of the property.
> What run-time use cases do you have in mind?

Allowing class loaders and other run-time tools to query the annotation 
value and take some action, like log a warning or potentially refuse to 
link.

>
>>> - The annotation is in the top-level "jdk" package.  What's the
>>>    rationale for this?  I'd have expected it to be defined in
>>>    "jdk.annotations", so that if and when other JDK-specific
>>>    annotations arise we have one convenient place to find them,
>>>    and only them.
>> There are 81 subtypes of java.lang.annotation.Annotation listed in JDK 8 b77
>>
>> ...
>>
>> That gives a total of 42 annotation types defined in packages ending
>> with "annotation" or about half of them. However, I would discount
>> java.lang.annotation and javax.xml.bind.annotation as outliers, in which
>> case most JDK annotations are *not* in a dedicated package.
>>
>> I think it is usually not helpful to segregate annotation types into
>> dedicated packages, after all we don't have "enums", "interfaces", and
>> "classes" packages and there are nearly as many annotations defined
>> directly in java.lang (SuppressWarnings, Deprecated, Override,
>> SafeVarargs, etc.) as in java.lang.annotation. ...
> Fair enough.  What struck me as odd about "jdk.Supported" is that it's
> a type in a top-level package, which is not something we've ever had
> before.  It's a bit jarring, though not illogical, so I suppose I can
> get used to it.

As an aside, going forward I think we should make greater use of the 
"jdk.*" namespace for JDK-specific types. The JDK codebase has outlasted 
Sun Microsystems (R.I.P.) and therefore the natural lifetime of 
"com.sun.*." APIs. The JDK has also outlasted both of Sun Microsystems' 
ticker symbols (SUNW, and JAVA); as I understand their conventions, 
ticker symbols are a preferred component of Solaris package names. The 
"jdk" name will be appropriate as long as the JDK is around.


>
> I did just notice that the annotation's source file is in the langtools
> repo rather than the jdk repo.  What's the rationale for that?  I think
> most JDK developers would expect to find it in the jdk repo.

As covered in other responses, while the jdk repo is the natural home, 
langtools was for bootstrapping reasons.

Cheers,

-Joe