On Mon, 17 Apr 2023 20:46:36 GMT, Doug Simon <dnsi...@openjdk.org> wrote:
> > From the long-term perspective, it is likely that the set of kinds of
> > elements that can occur in an annotation will be expanded, for example,
> > method references are a repeated request. Easing future maintenance to
> > gives more inter-source linkage in this situation and error handling for
> > this case in the libgraal code would be prudent IMO.
>
> I'm not sure what you're suggesting in terms of how I should update this PR.
> Do you mean `AnnotationData.get` needs to somehow be more flexible? If so,
> could you please give a concrete example of what you're after.
Let me explain my concerns in more detail.
We have (at least) two separate annotations implementations in the JDK, one in
javac for compile-time and other in core reflection for runtime. Due to various
technical constraints, the implementations are necessarily separate (although
the annotation objects constructed by javac do also implement the
java.lang.annotation.Annotation interface also used by core reflection).
This work is proposing to add another partial implementation to satisfy other
technical goals, reusing portions of the existing core reflection machinery.
If at some point the universe of objects that can be encoded as annotation is
expanded, e..g method literals as mentioned previously, it is well-understood
that core reflection and javac will need to be updated. I think it would be
relatively easy to overlook the need to make corresponding updates to libgraal
API and all regression tests might still pass.
As a concrete suggestion, if such an unknown annotation was handed over to
libgraal, I think the code should reject it (AssertionError("unexpected
annotation component") etc.) in hope of the omission getting corrected sooner.
Also, leaving some bread crumb comments to future maintainers of core
reflection annotations in that implementation package would be helpful too,
e.g. "// used by libgraal".
HTH
-------------
PR Comment: https://git.openjdk.org/jdk/pull/12810#issuecomment-1512343062
