avoiding unnecessary second Cast in instanceof patterns

[mpalat]

refer @stephan-herrmann 's comment at: https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues/2358#issuecomment-2359108740 " oking at AnnotationCodeMiningProvider we are talking about this code:

    if (getAdapter(IAnnotationAccess.class) instanceof IAnnotationAccessExtension annotationAccessExtension) {
        this.annotationAccess= annotationAccessExtension;
    } else {
        throw new IllegalStateException("annotationAccess must implement IAnnotationAccessExtension"); //$NON-NLS-1$
    }

Here getAdapter(Class) is declared to return . With T=IAnnotationAccess a checkcast to the statically known type is correct. I only wonder, if previously implementation of instanceof patterns took care to avoid unnecessary casts, when the only use of that value would undergo a second checkcast immediately after. -.. was any such optimization intended at some point? "

Also ref the comment below as well: public T getAdapter(Class adapter) { if (adapter == X509Certificate.class && !certificates.isEmpty()) { if (certificates.get(0) instanceof X509Certificate certificate) { return adapter.cast(certificate); } } The result of certificates.get(0) gets casted twice.

To check if such an optimization can be implemented

[stephan-herrmann]

To check if such an optimization can be implemented

My original question was: "was any such optimization intended at some point?" Meaning: did recent feature work break existing optimization?

[mpalat]

To check if such an optimization can be implemented

My original question was: "was any such optimization intended at some point?" Meaning: did recent feature work break existing optimization?

There was no optimization intended earlier - the recent work did not break any existing optimization. This issue is to check whether such an optimization can be done without too much hassle. btw, I would rate this a lower prio issue for now

[srikanth-sankaran]

I can investigate this one.

[stephan-herrmann]

There was no optimization intended earlier

In that case I suspect the following: prior to JEP 455 the LHS of instanceof simply never required a cast, because instanceof is applicable to all reference types.

With JEP 455 I recall situations like this requiring indeed one more cast:

List<Integer> ints = ...;
if (ints.get(0) instanceof int i) ...

At bytecode level, ints.get(0) returns java/lang/Object, but from the type argument the compiler knows that the value will indeed be an Integer so a checkcast java/lang/Integer is safe. After that we can insert an unboxing operation and assign the result to the pattern variable i. Without the intermediate cast to Integer this unboxing would not be possible, i.e., pattern matching wants to work from the "real" static type of its LHS, not the erasure.

From the p.o.v. of the pattern, this checkcast is not part of the testing conversion, so this is indeed a part of translating ints.get(0), rather than belonging to the pattern.

So the question could be rephrased: can we (without undue complexity) recognize whether or not the unerased type is actually needed / used?

With only two hits in the entire SDK, the possible gain will probably not be huge ...

[mpalat]

From the p.o.v. of the pattern, this checkcast is not part of the testing conversion, so this is indeed a part of translating ints.get(0), rather than belonging to the pattern.

yes, it is not part of testing conversion

So the question could be rephrased: can we (without undue complexity) recognize whether or not the unerased type is actually needed / used?

With only two hits in the entire SDK, the possible gain will probably not be huge ...

Agree on this part - I did some initial investigation on this whether to club it together but it was not straightforward. @srikanth-sankaran please take this only as low priority if you ever get to this.