You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by jwagenleitner <gi...@git.apache.org> on 2016/02/05 02:41:42 UTC
[GitHub] groovy pull request: GROOVY-7742 - Stackoverflow when method param...
GitHub user jwagenleitner opened a pull request:
https://github.com/apache/groovy/pull/253
GROOVY-7742 - Stackoverflow when method parameters have different placeholders
Not sure about this fix, but putting it out there for review.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jwagenleitner/groovy GROOVY-7742
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/groovy/pull/253.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #253
----
commit 600b31f1277724c286ebc916c5032e8f5825fcd6
Author: John Wagenleitner <jw...@apache.org>
Date: 2016-02-05T01:39:33Z
GROOVY-7742 - Stackoverflow when method parameters have different placeholders
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: GROOVY-7742 - Stackoverflow when method param...
Posted by shils <gi...@git.apache.org>.
Github user shils commented on a diff in the pull request:
https://github.com/apache/groovy/pull/253#discussion_r52126075
--- Diff: src/test/groovy/transform/stc/GenericsSTCTest.groovy ---
@@ -1800,6 +1800,19 @@ assert result == 'ok'
'''
}
+ // GROOVY-7742
+ void testMethodParamWithDifferentPlaceholderForTypeArgument() {
+ // We use <T> here while List uses <E>
--- End diff --
That was my thought initially as well, but in that case I would expect the following to fail since AtomicReference uses 'V' as a placeholder:
```
import java.util.concurrent.atomic.AtomicReference
@groovy.transform.CompileStatic
class Foo {
public <V> AtomicReference<V> firstReference(List<AtomicReference<V>> references) {
return references.first()
}
}
```
The code above compiles but, interestingly enough, the variant below does not:
```
import java.util.concurrent.atomic.AtomicReference
@groovy.transform.CompileStatic
class Foo {
public <T> AtomicReference<T> firstClass(List<AtomicReference<T>> references) {
return references.first()
}
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: GROOVY-7742 - Stackoverflow when method param...
Posted by jwagenleitner <gi...@git.apache.org>.
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/253#discussion_r52127643
--- Diff: src/test/groovy/transform/stc/GenericsSTCTest.groovy ---
@@ -1800,6 +1800,19 @@ assert result == 'ok'
'''
}
+ // GROOVY-7742
+ void testMethodParamWithDifferentPlaceholderForTypeArgument() {
+ // We use <T> here while List uses <E>
--- End diff --
Looks like when it resolves the placeholders it adds T based on DGM `static <T> T first(List<T> self)` since it uses `T` as placeholder. The real code that produced this error used `collect` which also uses `T`. Changing the code to use `return classes.get(0)` fixes it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: GROOVY-7742 - Stackoverflow when method param...
Posted by fpavageau <gi...@git.apache.org>.
Github user fpavageau commented on a diff in the pull request:
https://github.com/apache/groovy/pull/253#discussion_r52125721
--- Diff: src/test/groovy/transform/stc/GenericsSTCTest.groovy ---
@@ -1800,6 +1800,19 @@ assert result == 'ok'
'''
}
+ // GROOVY-7742
+ void testMethodParamWithDifferentPlaceholderForTypeArgument() {
+ // We use <T> here while List uses <E>
--- End diff --
The problem does not appear when using a different placeholder than `List` (i.e. not `E`), it appears when using the same as `Class` (`T`).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: GROOVY-7742 - Stackoverflow when method param...
Posted by shils <gi...@git.apache.org>.
Github user shils commented on a diff in the pull request:
https://github.com/apache/groovy/pull/253#discussion_r52125211
--- Diff: src/test/groovy/transform/stc/GenericsSTCTest.groovy ---
@@ -1800,6 +1800,19 @@ assert result == 'ok'
'''
}
+ // GROOVY-7742
+ void testMethodParamWithDifferentPlaceholderForTypeArgument() {
+ // We use <T> here while List uses <E>
--- End diff --
I don't think using a different placeholder than the generic type is the issue here e.g. if 'T' is replaced with 'U' in this test, it passes without your change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: GROOVY-7742 - Stackoverflow when method param...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/groovy/pull/253
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---