[
https://issues.apache.org/jira/browse/GROOVY-12584?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King resolved GROOVY-12584.
--------------------------------
Fix Version/s: 7.0.0-alpha-1
6.0.1
Resolution: Fixed
> 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
> Affects Versions: 4.0.33, 5.1.3, 6.0.0
> Reporter: Paul King
> Assignee: Paul King
> Priority: Minor
> Fix For: 7.0.0-alpha-1, 6.0.1
>
>
> 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)
