[
https://issues.apache.org/jira/browse/GROOVY-12584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135433#comment-18135433
]
ASF GitHub Bot commented on GROOVY-12584:
-----------------------------------------
codecov-commenter commented on PR #3111:
URL: https://github.com/apache/groovy/pull/3111#issuecomment-6534581924
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3111?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 73.7735%. Comparing base
([`396e862`](https://app.codecov.io/gh/apache/groovy/commit/396e862eda6c8a2fddaeb3eaf58e86cb5e6f08bc?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`920ed21`](https://app.codecov.io/gh/apache/groovy/commit/920ed2107d8c23f08a89775aa876e6a4c88ea6b8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/3111?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #3111 +/- ##
==================================================
- Coverage 73.7763% 73.7735% -0.0028%
+ Complexity 42753 42750 -3
==================================================
Files 1678 1678
Lines 147462 147454 -8
Branches 27899 27894 -5
==================================================
- Hits 108792 108782 -10
- Misses 28513 28515 +2
Partials 10157 10157
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3111?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...vy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java](https://app.codecov.io/gh/apache/groovy/pull/3111?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FIndyGuardsFiltersAndSignatures.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L0luZHlHdWFyZHNGaWx0ZXJzQW5kU2lnbmF0dXJlcy5qYXZh)
| `91.0256% <ø> (ø)` | |
|
[...java/org/codehaus/groovy/vmplugin/v8/Selector.java](https://app.codecov.io/gh/apache/groovy/pull/3111?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FSelector.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L1NlbGVjdG9yLmphdmE=)
| `81.8336% <ø> (+0.5941%)` | :arrow_up: |
... and [14 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/3111/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> 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
> Priority: Minor
>
> 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)
