Paul King created GROOVY-12584:
----------------------------------
Summary: Indy cast of a Collection to an abstract subclass of
HashSet or ArrayList throws a bare ClassCastException
Key: GROOVY-12584
URL: https://issues.apache.org/jira/browse/GROOVY-12584
Project: Groovy
Issue Type: Bug
Components: Compiler
Reporter: Paul King
h3. Problem
The invokedynamic cast selector (\{{Selector.CastSelector.handleCollections}})
has a fast path that is meant to turn a \{{Collection}} into a \{{HashSet}} or
\{{ArrayList}} for an abstract target type. Its check is the wrong way round:
{code:java}
private static boolean isAbstractClassOf(final Class<?> type, final Class<?>
callSiteType) {
return (callSiteType.isInterface() ||
Modifier.isAbstract(callSiteType.getModifiers())) &&
type.isAssignableFrom(callSiteType);
}
{code}
{\{isAbstractClassOf(HashSet.class, target)}} is true only when the target is
an abstract *subclass* of \{{HashSet}}. So for the targets it was presumably
written for (\{{Set}}, \{{AbstractSet}}, \{{Collection}}, \{{List}}) the fast
path never fires, and the cast goes to
\{{DefaultTypeTransformation.castToType}}, as it does with classic call sites.
For an abstract subclass of \{{HashSet}} or \{{ArrayList}} it does fire. It
builds a plain \{{HashSet}}/\{{ArrayList}}, which then fails the cast to the
target type with a bare \{{ClassCastException}}, where classic call sites throw
\{{GroovyCastException}}.
h3. Reproducer
{code:groovy}
abstract class ASet extends HashSet {}
abstract class AList extends ArrayList {}
Object list = [3, 1, 2, 1]
Object set = [3, 1, 2] as LinkedHashSet
ASet s = list // or: AList l = set
{code}
||Cast||indy||classic call sites||
|\{{ASet s = list}}|\{{ClassCastException: Cannot cast java.util.HashSet to
ASet}}|\{{GroovyCastException: ... Could not find matching constructor for:
ASet(Integer, Integer, Integer, Integer)}}|
|\{{AList l = set}}|\{{ClassCastException: Cannot cast java.util.ArrayList to
AList}}|\{{GroovyCastException: ... Could not find matching constructor for:
AList(Integer, Integer, Integer)}}|
|\{{Set s = list}}, \{{AbstractSet s = list}}|\{{LinkedHashSet [3, 1, 2]}}|same|
|\{{List l = set}}|\{{GroovyCastException}}|same|
The cast fails either way; only the exception type and message differ.
\{{GroovyCastException}} extends \{{ClassCastException}}, so existing
\{{catch}} blocks are unaffected by a fix.
The code dates back to the early indy work and is identical on
\{{GROOVY_2_5_X}} (\{{vmplugin.v7}}), \{{GROOVY_3_0_X}}, \{{GROOVY_4_0_X}},
\{{GROOVY_5_1_X}}, \{{GROOVY_6_0_X}} and master.
h3. Proposed fix
Remove \{{handleCollections()}} and \{{isAbstractClassOf()}} so that every
collection cast goes through \{{castToType}}, matching classic call sites.
Correcting the check instead would make indy diverge from classic call sites:
* \{{Set s = list}} would produce a \{{HashSet}} (losing order) instead of
today's \{{LinkedHashSet}}.
* \{{List l = set}} would start succeeding, where it currently throws
\{{GroovyCastException}} on both paths.
Removing the fast path also removes a latent hazard: the cached handle \{{new
HashSet(collection)}} is not null-safe, and since GROOVY-12557 a cast from a
final static type is no longer guarded against a later \{{null}} value.
{\{IndyGuardsFiltersAndSignatures.HASHSET_CONSTRUCTOR}} and
\{{ARRAYLIST_CONSTRUCTOR}} are public. Keep them, deprecated, on master; they
would then have no in-tree users.
A test should cover both abstract-subclass casts expecting
\{{GroovyCastException}} under indy, plus the
\{{Set}}/\{{AbstractSet}}/\{{Collection}} shapes to confirm the result types
don't change.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
