You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by danielsun1106 <gi...@git.apache.org> on 2018/12/10 19:07:16 UTC
[GitHub] groovy pull request #837: GROOVY-8917: Failed to infer parameter type of som...
GitHub user danielsun1106 opened a pull request:
https://github.com/apache/groovy/pull/837
GROOVY-8917: Failed to infer parameter type of some SAM, e.g. BinaryO…
…perator
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/danielsun1106/groovy refine-type-inference
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/groovy/pull/837.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 #837
----
commit 3669f04205265b62e3a3f339aa1b81fbf3003ecb
Author: danielsun1106 <re...@...>
Date: 2018-12-10T19:06:29Z
GROOVY-8917: Failed to infer parameter type of some SAM, e.g. BinaryOperator
----
---
[GitHub] groovy pull request #837: GROOVY-8917: Failed to infer parameter type of som...
Posted by danielsun1106 <gi...@git.apache.org>.
Github user danielsun1106 commented on a diff in the pull request:
https://github.com/apache/groovy/pull/837#discussion_r240451854
--- Diff: src/main/java/org/codehaus/groovy/classgen/asm/util/TypeUtil.java ---
@@ -213,4 +220,15 @@ private static String makeRefDescription(String name) {
long.class, Long.class,
short.class, Short.class
);
+
+ private static final Map<ClassNode, ClassNode> PRIMITIVE_CLASSNODE_TO_WRAPPED_CLASSNODE_MAP = Maps.of(
+ ClassHelper.byte_TYPE, ClassHelper.Byte_TYPE,
+ ClassHelper.boolean_TYPE, ClassHelper.Boolean_TYPE,
+ ClassHelper.char_TYPE, ClassHelper.Character_TYPE,
+ ClassHelper.double_TYPE, ClassHelper.Double_TYPE,
+ ClassHelper.float_TYPE, ClassHelper.Float_TYPE,
+ ClassHelper.int_TYPE, ClassHelper.Integer_TYPE,
+ ClassHelper.long_TYPE, ClassHelper.Long_TYPE,
+ ClassHelper.short_TYPE, ClassHelper.Short_TYPE
+ );
--- End diff --
Gotcha. I will use the map you suggest instead.
---
[GitHub] groovy pull request #837: GROOVY-8917: Failed to infer parameter type of som...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/groovy/pull/837
---
[GitHub] groovy pull request #837: GROOVY-8917: Failed to infer parameter type of som...
Posted by danielsun1106 <gi...@git.apache.org>.
Github user danielsun1106 commented on a diff in the pull request:
https://github.com/apache/groovy/pull/837#discussion_r240468415
--- Diff: src/main/java/org/codehaus/groovy/classgen/asm/util/TypeUtil.java ---
@@ -213,4 +220,15 @@ private static String makeRefDescription(String name) {
long.class, Long.class,
short.class, Short.class
);
+
+ private static final Map<ClassNode, ClassNode> PRIMITIVE_CLASSNODE_TO_WRAPPED_CLASSNODE_MAP = Maps.of(
+ ClassHelper.byte_TYPE, ClassHelper.Byte_TYPE,
+ ClassHelper.boolean_TYPE, ClassHelper.Boolean_TYPE,
+ ClassHelper.char_TYPE, ClassHelper.Character_TYPE,
+ ClassHelper.double_TYPE, ClassHelper.Double_TYPE,
+ ClassHelper.float_TYPE, ClassHelper.Float_TYPE,
+ ClassHelper.int_TYPE, ClassHelper.Integer_TYPE,
+ ClassHelper.long_TYPE, ClassHelper.Long_TYPE,
+ ClassHelper.short_TYPE, ClassHelper.Short_TYPE
+ );
--- End diff --
@paulk-asert I've refined the PR according to your suggestion ;-)
---
[GitHub] groovy pull request #837: GROOVY-8917: Failed to infer parameter type of som...
Posted by paulk-asert <gi...@git.apache.org>.
Github user paulk-asert commented on a diff in the pull request:
https://github.com/apache/groovy/pull/837#discussion_r240451547
--- Diff: src/main/java/org/codehaus/groovy/classgen/asm/util/TypeUtil.java ---
@@ -213,4 +220,15 @@ private static String makeRefDescription(String name) {
long.class, Long.class,
short.class, Short.class
);
+
+ private static final Map<ClassNode, ClassNode> PRIMITIVE_CLASSNODE_TO_WRAPPED_CLASSNODE_MAP = Maps.of(
+ ClassHelper.byte_TYPE, ClassHelper.Byte_TYPE,
+ ClassHelper.boolean_TYPE, ClassHelper.Boolean_TYPE,
+ ClassHelper.char_TYPE, ClassHelper.Character_TYPE,
+ ClassHelper.double_TYPE, ClassHelper.Double_TYPE,
+ ClassHelper.float_TYPE, ClassHelper.Float_TYPE,
+ ClassHelper.int_TYPE, ClassHelper.Integer_TYPE,
+ ClassHelper.long_TYPE, ClassHelper.Long_TYPE,
+ ClassHelper.short_TYPE, ClassHelper.Short_TYPE
+ );
--- End diff --
I was referring to `ClassHelper#PRIMITIVE_TYPE_TO_WRAPPER_TYPE_MAP` not `TypeUtil#PRIMITIVE_TYPE_TO_WRAPPED_CLASS_MAP`.
---
[GitHub] groovy pull request #837: GROOVY-8917: Failed to infer parameter type of som...
Posted by paulk-asert <gi...@git.apache.org>.
Github user paulk-asert commented on a diff in the pull request:
https://github.com/apache/groovy/pull/837#discussion_r240369943
--- Diff: src/main/java/org/codehaus/groovy/classgen/asm/util/TypeUtil.java ---
@@ -213,4 +220,15 @@ private static String makeRefDescription(String name) {
long.class, Long.class,
short.class, Short.class
);
+
+ private static final Map<ClassNode, ClassNode> PRIMITIVE_CLASSNODE_TO_WRAPPED_CLASSNODE_MAP = Maps.of(
+ ClassHelper.byte_TYPE, ClassHelper.Byte_TYPE,
+ ClassHelper.boolean_TYPE, ClassHelper.Boolean_TYPE,
+ ClassHelper.char_TYPE, ClassHelper.Character_TYPE,
+ ClassHelper.double_TYPE, ClassHelper.Double_TYPE,
+ ClassHelper.float_TYPE, ClassHelper.Float_TYPE,
+ ClassHelper.int_TYPE, ClassHelper.Integer_TYPE,
+ ClassHelper.long_TYPE, ClassHelper.Long_TYPE,
+ ClassHelper.short_TYPE, ClassHelper.Short_TYPE
+ );
--- End diff --
Isn't this already in ClassHelper#PRIMITIVE_TYPE_TO_WRAPPER_TYPE_MAP?
---
[GitHub] groovy pull request #837: GROOVY-8917: Failed to infer parameter type of som...
Posted by paulk-asert <gi...@git.apache.org>.
Github user paulk-asert commented on a diff in the pull request:
https://github.com/apache/groovy/pull/837#discussion_r240451723
--- Diff: src/main/java/org/codehaus/groovy/classgen/asm/util/TypeUtil.java ---
@@ -213,4 +220,15 @@ private static String makeRefDescription(String name) {
long.class, Long.class,
short.class, Short.class
);
+
+ private static final Map<ClassNode, ClassNode> PRIMITIVE_CLASSNODE_TO_WRAPPED_CLASSNODE_MAP = Maps.of(
+ ClassHelper.byte_TYPE, ClassHelper.Byte_TYPE,
+ ClassHelper.boolean_TYPE, ClassHelper.Boolean_TYPE,
+ ClassHelper.char_TYPE, ClassHelper.Character_TYPE,
+ ClassHelper.double_TYPE, ClassHelper.Double_TYPE,
+ ClassHelper.float_TYPE, ClassHelper.Float_TYPE,
+ ClassHelper.int_TYPE, ClassHelper.Integer_TYPE,
+ ClassHelper.long_TYPE, ClassHelper.Long_TYPE,
+ ClassHelper.short_TYPE, ClassHelper.Short_TYPE
+ );
--- End diff --
And I guess also `TypeUtil#PRIMITIVE_TYPE_TO_WRAPPED_CLASS_MAP` would be better named `PRIMITIVE_CLASS_TO_WRAPPED_CLASS_MAP`.
---
[GitHub] groovy pull request #837: GROOVY-8917: Failed to infer parameter type of som...
Posted by paulk-asert <gi...@git.apache.org>.
Github user paulk-asert commented on a diff in the pull request:
https://github.com/apache/groovy/pull/837#discussion_r240451187
--- Diff: src/main/java/org/codehaus/groovy/classgen/asm/util/TypeUtil.java ---
@@ -213,4 +220,15 @@ private static String makeRefDescription(String name) {
long.class, Long.class,
short.class, Short.class
);
+
+ private static final Map<ClassNode, ClassNode> PRIMITIVE_CLASSNODE_TO_WRAPPED_CLASSNODE_MAP = Maps.of(
+ ClassHelper.byte_TYPE, ClassHelper.Byte_TYPE,
+ ClassHelper.boolean_TYPE, ClassHelper.Boolean_TYPE,
+ ClassHelper.char_TYPE, ClassHelper.Character_TYPE,
+ ClassHelper.double_TYPE, ClassHelper.Double_TYPE,
+ ClassHelper.float_TYPE, ClassHelper.Float_TYPE,
+ ClassHelper.int_TYPE, ClassHelper.Integer_TYPE,
+ ClassHelper.long_TYPE, ClassHelper.Long_TYPE,
+ ClassHelper.short_TYPE, ClassHelper.Short_TYPE
+ );
--- End diff --
Both appear to be Map<ClassNode, ClassNode> to me?
---
[GitHub] groovy pull request #837: GROOVY-8917: Failed to infer parameter type of som...
Posted by danielsun1106 <gi...@git.apache.org>.
Github user danielsun1106 commented on a diff in the pull request:
https://github.com/apache/groovy/pull/837#discussion_r240445739
--- Diff: src/main/java/org/codehaus/groovy/classgen/asm/util/TypeUtil.java ---
@@ -213,4 +220,15 @@ private static String makeRefDescription(String name) {
long.class, Long.class,
short.class, Short.class
);
+
+ private static final Map<ClassNode, ClassNode> PRIMITIVE_CLASSNODE_TO_WRAPPED_CLASSNODE_MAP = Maps.of(
+ ClassHelper.byte_TYPE, ClassHelper.Byte_TYPE,
+ ClassHelper.boolean_TYPE, ClassHelper.Boolean_TYPE,
+ ClassHelper.char_TYPE, ClassHelper.Character_TYPE,
+ ClassHelper.double_TYPE, ClassHelper.Double_TYPE,
+ ClassHelper.float_TYPE, ClassHelper.Float_TYPE,
+ ClassHelper.int_TYPE, ClassHelper.Integer_TYPE,
+ ClassHelper.long_TYPE, ClassHelper.Long_TYPE,
+ ClassHelper.short_TYPE, ClassHelper.Short_TYPE
+ );
--- End diff --
@paulk-asert As we can see, the existing type mapping is Class to Class, but we need map ClassNode to ClassNode. Though we can convert ClassNode to Class and get the mapping result then convert the result to ClassNode, it is less efficient.
https://github.com/apache/groovy/blob/9cea59b95367c1d563e719ebeddcdecb58199750/src/main/java/org/codehaus/groovy/classgen/asm/util/TypeUtil.java#L213
---