You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2026/05/02 06:03:00 UTC
[jira] [Commented] (GROOVY-11375) Power binary expression finds incorrect type - currently uses declaration inferred type
[ https://issues.apache.org/jira/browse/GROOVY-11375?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18077774#comment-18077774 ]
ASF GitHub Bot commented on GROOVY-11375:
-----------------------------------------
Copilot commented on code in PR #2311:
URL: https://github.com/apache/groovy/pull/2311#discussion_r3176226386
##########
src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java:
##########
@@ -32,20 +32,23 @@
* type information from node metadata generated by the static type checker.
*/
public class StaticTypesTypeChooser extends StatementMetaTypeChooser {
+
@Override
public ClassNode resolveType(final Expression exp, final ClassNode current) {
- ASTNode target = getTarget(exp); // see GROOVY-9344, GROOVY-9607
-
- ClassNode inferredType = target.getNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE);
+ ClassNode inferredType = exp.getNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE);
+ if (inferredType == null) {
+ inferredType = exp.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
+ }
if (inferredType == null) {
- inferredType = target.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
+ ASTNode node = getTarget(exp); // GROOVY-9344, GROOVY-9607
+ inferredType = node.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
Review Comment:
This drops the fallback to `DECLARATION_INFERRED_TYPE` on the accessed target. Fresh `VariableExpression` wrappers such as `varX(variable)` in `ClosureWriter#getClosureSharedVariables` don't carry metadata on the wrapper itself, so after this change captured/aliased variables resolve to the target's current `INFERRED_TYPE` instead of the declaration/LUB type. That can generate overly narrow bytecode types for closure-shared variables and reintroduce the kind of regressions the old target lookup was protecting against.
##########
src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy:
##########
@@ -242,23 +241,29 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
'''
}
- // GROOVY-10096
- @NotYetImplemented
+ // GROOVY-11769
void testInstanceOf10() {
Review Comment:
Repurposing this test removes the only regression coverage for GROOVY-10096 from the suite. A new GROOVY-11769 test should be added under a different name so we don't stop checking the previous return-type inference bug.
##########
src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy:
##########
@@ -242,23 +241,29 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
'''
}
- // GROOVY-10096
- @NotYetImplemented
+ // GROOVY-11769
void testInstanceOf10() {
- shouldFailWithMessages '''
- class Foo {
- void foo() {
- }
+ assertScript '''
+ abstract class Foo {
+ abstract boolean isBaz()
}
class Bar extends Foo {
- void bar() {
- }
+ final boolean baz = false
}
- static Bar baz(Foo foo) {
- (false || foo instanceof Bar) ? foo : new Bar()
+ class Baz extends Foo {
+ final boolean baz = true
}
- ''',
- 'Cannot return value of type Foo for method returning Bar'
+
+ void test(Foo foo) {
+ if (foo instanceof Bar || foo.isBaz()) {
+ foo.toString()
Review Comment:
This assertion doesn't actually verify the `instanceof` flow typing change: `foo.toString()` is available on `Object`, so the test will pass even if `foo` is never narrowed after the `foo instanceof Bar || foo.isBaz()` condition. Please assert behavior that requires the narrowed type; otherwise this regression test won't catch the bug it is meant to cover.
> Power binary expression finds incorrect type - currently uses declaration inferred type
> ---------------------------------------------------------------------------------------
>
> Key: GROOVY-11375
> URL: https://issues.apache.org/jira/browse/GROOVY-11375
> Project: Groovy
> Issue Type: Bug
> Components: Static compilation
> Affects Versions: 5.0.0-alpha-8, 4.0.21
> Reporter: Paul King
> Assignee: Eric Milles
> Priority: Major
> Fix For: 5.0.0-alpha-9
>
>
> For this code:
> {code}
> @groovy.transform.CompileStatic
> def method() {
> var temp = 'foo'
> temp = temp.size()
> assert temp ** temp == 27
> }
> method()
> {code}
> The compiler gives this error:
> {noformat}
> BUG! exception in phase 'class generation' in source unit 'ConsoleScript3' At line 10 column 8
> On receiver: temp with message: power and arguments: temp
> This method should not have been called. Please try to create a simple example reproducing
> this error and file a bug report at https://issues.apache.org/jira/browse/GROOVY
> at org.codehaus.groovy.classgen.asm.sc.StaticTypesCallSiteWriter.makeSingleArgumentCall(StaticTypesCallSiteWriter.java:582)
> at org.codehaus.groovy.classgen.asm.InvocationWriter.makeSingleArgumentCall(InvocationWriter.java:626)
> at org.codehaus.groovy.classgen.asm.BinaryExpressionHelper.evaluateBinaryExpression(BinaryExpressionHelper.java:602)
> at org.codehaus.groovy.classgen.asm.BinaryExpressionMultiTypeDispatcher.evaluateBinaryExpression(BinaryExpressionMultiTypeDispatcher.java:242)
> at org.codehaus.groovy.classgen.asm.BinaryExpressionHelper.eval(BinaryExpressionHelper.java:264)
> at org.codehaus.groovy.classgen.AsmClassGenerator.visitBinaryExpression(AsmClassGenerator.java:849)
> ...
> {noformat}
> We apply flow typing for the receiver but not the argument type. This looks like a case that should be handled where we currently have "TODO: more cases ..." in StaticTypesCallSiteWriter.
> The same situation occurs for the shift operators and plus etc., e.g.:
> {code}
> assert temp << temp == 24
> assert temp + temp == 6
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)