On Mon, 6 Feb 2023 13:55:59 GMT, Adam Sotona <asot...@openjdk.org> wrote:
>> src/java.base/share/classes/jdk/internal/classfile/AttributedElement.java
>> line 94:
>>
>>> 92: * are permitted.
>>> 93: */
>>> 94: enum Kind {
>>
>> Not sure how to interpret this. This seems to refer to an attribute - e.g.
>> where is an attribute allowed - rather than to the element (e.g. the
>> declaration to which the attribute is attached). All the constants are
>> unused, so hard to make sense of how this is used.
>
> There are at least 72 usages of AttributedElement.Kind across the Classfile
> API.
> Do you suggest to rename it to something else (for example Location)?
Uhm - I can't see these usages... something seems to be off with my IDE
configuration. I did a grep and I now saw the uses. That said, having the
Kind/Location inside AttributedElement still looks weird to me. The "places
where an attribute can appear" is a property of an `Attribute`, not of an
attributed element (which is used to model elements which can have attributes).
>> src/java.base/share/classes/jdk/internal/classfile/Attributes.java line 774:
>>
>>> 772: */
>>> 773: public static AttributeMapper<?> standardAttribute(Utf8Entry name)
>>> {
>>> 774: int hash = name.hashCode();
>>
>> If we have a map, why do we need this "inlined" map here? Performance
>> reasons?
>
> Yes, performance is the main reason.
> I'll note to do a fresh differential performance benchmarks with a HashMap.
thanks
-------------
PR: https://git.openjdk.org/jdk/pull/10982
