From commits-return-8429-archive-asf-public=cust-asf.ponee.io@groovy.apache.org Wed May 1 12:40:45 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 7750E180629 for ; Wed, 1 May 2019 14:40:45 +0200 (CEST) Received: (qmail 26801 invoked by uid 500); 1 May 2019 12:40:44 -0000 Mailing-List: contact commits-help@groovy.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@groovy.apache.org Delivered-To: mailing list commits@groovy.apache.org Received: (qmail 26790 invoked by uid 99); 1 May 2019 12:40:44 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 01 May 2019 12:40:44 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id AA45587145; Wed, 1 May 2019 12:40:39 +0000 (UTC) Date: Wed, 01 May 2019 12:40:39 +0000 To: "commits@groovy.apache.org" Subject: [groovy] branch master updated: GROOVY-9086: @CompileStatic of closure calls with OWNER_FIRST fail at runtime with ClassCastException (closes #917) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <155671443956.21485.17874238387307984834@gitbox.apache.org> From: paulk@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: groovy X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 7b1ab21e2839ab7b5662cced04d11bdd152371e7 X-Git-Newrev: 5d529da542befe55032e87de1f71a5010a97310b X-Git-Rev: 5d529da542befe55032e87de1f71a5010a97310b X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. paulk pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/groovy.git The following commit(s) were added to refs/heads/master by this push: new 5d529da GROOVY-9086: @CompileStatic of closure calls with OWNER_FIRST fail at runtime with ClassCastException (closes #917) 5d529da is described below commit 5d529da542befe55032e87de1f71a5010a97310b Author: Paul King AuthorDate: Wed May 1 06:52:35 2019 +1000 GROOVY-9086: @CompileStatic of closure calls with OWNER_FIRST fail at runtime with ClassCastException (closes #917) --- .../transform/stc/StaticTypeCheckingVisitor.java | 111 +++++++------------ src/test/gls/closures/ResolveStrategyTest.groovy | 120 +++++++++++++++++++++ 2 files changed, 157 insertions(+), 74 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index d531948..3d4dc78 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -1857,7 +1857,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { if (visitor != null) visitor.visitProperty(propertyNode); storeWithResolve(propertyNode.getOriginType(), receiver, propertyNode.getDeclaringClass(), propertyNode.isStatic(), expressionToStoreOn); if (delegationData != null) { - delegationData = adjustData(delegationData, receiver, typeCheckingContext.delegationMetadata); expressionToStoreOn.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData); } return true; @@ -3281,53 +3280,51 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } } - private static boolean isTraitHelper(ClassNode node) { - return node instanceof InnerClassNode && Traits.isTrait(node.getOuterClass()); - } - protected void addReceivers(final List> receivers, final Collection> owners, final boolean implicitThis) { - if (typeCheckingContext.delegationMetadata == null || !implicitThis) { + if (!implicitThis || typeCheckingContext.delegationMetadata == null) { receivers.addAll(owners); - return; - } - - DelegationMetadata dmd = typeCheckingContext.delegationMetadata; - StringBuilder path = new StringBuilder(); - while (dmd != null) { - int strategy = dmd.getStrategy(); - ClassNode delegate = dmd.getType(); - dmd = dmd.getParent(); - switch (strategy) { - case Closure.OWNER_FIRST: - receivers.addAll(owners); - path.append("delegate"); - doAddDelegateReceiver(receivers, path, delegate); - break; - case Closure.DELEGATE_FIRST: - path.append("delegate"); - doAddDelegateReceiver(receivers, path, delegate); - receivers.addAll(owners); - break; - case Closure.OWNER_ONLY: + } else { + addReceivers(receivers, owners, typeCheckingContext.delegationMetadata, ""); + } + } + + private static void addReceivers(final List> receivers, + final Collection> owners, + final DelegationMetadata dmd, + final String path) { + int strategy = dmd.getStrategy(); + switch (strategy) { + case Closure.DELEGATE_ONLY: + case Closure.DELEGATE_FIRST: + addDelegateReceiver(receivers, dmd.getType(), path + "delegate"); + if (strategy == Closure.DELEGATE_FIRST) { + if (dmd.getParent() == null) { + receivers.addAll(owners); + } else { + addReceivers(receivers, owners, dmd.getParent(), path + "owner."); + } + } + break; + case Closure.OWNER_ONLY: + case Closure.OWNER_FIRST: + if (dmd.getParent() == null) { receivers.addAll(owners); - dmd = null; - break; - case Closure.DELEGATE_ONLY: - path.append("delegate"); - doAddDelegateReceiver(receivers, path, delegate); - dmd = null; - break; - } - path.append('.'); + } else { + addReceivers(receivers, owners, dmd.getParent(), path + "owner."); + } + if (strategy == Closure.OWNER_FIRST) { + addDelegateReceiver(receivers, dmd.getType(), path + "delegate"); + } + break; } } - private static void doAddDelegateReceiver(final List> receivers, final StringBuilder path, final ClassNode delegate) { - receivers.add(new Receiver(delegate, path.toString())); - if (isTraitHelper(delegate)) { - receivers.add(new Receiver(delegate.getOuterClass(), path.toString())); + private static void addDelegateReceiver(final List> receivers, final ClassNode delegate, final String path) { + receivers.add(new Receiver(delegate, path)); + if (Traits.isTrait(delegate.getOuterClass())) { + receivers.add(new Receiver(delegate.getOuterClass(), path)); } } @@ -3564,7 +3561,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } String data = chosenReceiver.getData(); if (data != null) { - data = adjustData(data, chosenReceiver.getType(), typeCheckingContext.delegationMetadata); // the method which has been chosen is supposed to be a call on delegate or owner // so we store the information so that the static compiler may reuse it call.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, data); @@ -3695,39 +3691,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { return newParameters; } - // adjust data to handle cases like nested .with since we didn't have enough information earlier - // TODO see if we can make the earlier detection smarter and then remove this adjustment - private static String adjustData(String data, ClassNode type, DelegationMetadata dmd) { - StringBuilder path = new StringBuilder(); - int i = 0; - String[] propertyPath = data.split("\\."); - while (dmd != null) { - int strategy = dmd.getStrategy(); - ClassNode delegate = dmd.getType(); - dmd = dmd.getParent(); - switch (strategy) { - case Closure.DELEGATE_FIRST: - if (!delegate.isDerivedFrom(CLOSURE_TYPE) && !delegate.isDerivedFrom(type)) { - path.append("owner"); // must be non-delegate case - } else { - path.append("delegate"); - } - break; - default: - if (i >= propertyPath.length) return data; - path.append(propertyPath[i]); - } - if (type.equals(delegate)) break; - i++; - if (dmd != null) path.append('.'); - } - String result = path.toString(); - if (!result.isEmpty()) { - return result; - } - return data; - } - /** * e.g. c(b(a())), a() and b() are nested method call, but c() is not * new C(b(a())) a() and b() are nested method call diff --git a/src/test/gls/closures/ResolveStrategyTest.groovy b/src/test/gls/closures/ResolveStrategyTest.groovy new file mode 100644 index 0000000..ea7cb8d --- /dev/null +++ b/src/test/gls/closures/ResolveStrategyTest.groovy @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package gls.closures + +import groovy.transform.CompileStatic +import static groovy.lang.Closure.* + +class ResolveStrategyTest extends GroovyTestCase { + void testDynamicSettingOfResolveStrategy() { + new MyClass().with { + assert run(DELEGATE_ONLY) == 12340000 // (*) + assert run(DELEGATE_FIRST) == 12040030 + assert run(OWNER_ONLY) == 1234 // (*) + assert run(OWNER_FIRST) == 41230 + + assert runOwnerOnly { m1() + m2() + m3() } == 1230 + assert runOwnerOnly { m1() + m2() + m3() + m4() } == 1234 // (*) + assert runOwnerFirst { m1() + m2() + m3() + m4() } == 41230 + assert runDelegateOnly { m1() + m2() + m3() + m4() } == 12340000 // (*) + assert runDelegateOnly { m1() + m2() + m4() } == 12040000 + assert runDelegateFirst { m1() + m2() + m3() + m4() } == 12340000 // (**) + + // nested cases + assert runDelegateFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 41230 + assert runOwnerFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12340000 // (**) + assert runOwnerFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 41230 + assert runDelegateFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12340000 // (**) + // (*) involves methodMissing as forced by ONLY strategy (no equivalent CS case below) + // (**) involves methodMissing since delegate methodMissing has precedence over explicit owner method + } + } + + @CompileStatic + void testStaticCases() { + new MyClass().with { + assert runOwnerOnly { m1() + m2() + m3() } == 1230 + assert runOwnerFirst { m1() + m2() + m3() + m4() } == 41230 + assert runDelegateOnly { m1() + m2() + m4() } == 12040000 + assert runDelegateFirst { m1() + m2() + m3() + m4() } == 12040030 // (*) + + // nested cases (GROOVY-9086) + assert runDelegateFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 12040030 // (*) + assert runOwnerFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12040030 // (*) + assert runOwnerFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 41230 + assert runDelegateFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12040030 // (*) + // (*) different to dynamic since static assumes no methodMissing + } + } +} + +class Delegate { + int m1() { 10000000 } + + int m2() { 2000000 } + + int m4() { 40000 } + + def methodMissing(String name, args) { + if (name.size() == 2) return 300000 + else throw new MissingMethodException(name, Delegate, args) + } +} + +class MyClass { + int m1() { 1000 } + + int m2() { 200 } + + int m3() { 30 } + + def methodMissing(String name, args) { 4 } + + def run(int rs) { + def answer = -1 + Closure c = { answer = m1() + m2() + m3() + m4() } + c.resolveStrategy = rs + c.delegate = new Delegate() + c() + answer + } + + def runDelegateFirst(@DelegatesTo(value = Delegate, strategy = DELEGATE_FIRST) Closure c) { + c.delegate = new Delegate() + c.resolveStrategy = DELEGATE_FIRST + c() + } + + def runDelegateOnly(@DelegatesTo(value = Delegate, strategy = DELEGATE_ONLY) Closure c) { + c.delegate = new Delegate() + c.resolveStrategy = DELEGATE_ONLY + c() + } + + def runOwnerFirst(@DelegatesTo(Delegate) Closure c) { + c.delegate = new Delegate() + c() + } + + def runOwnerOnly(@DelegatesTo(value = Delegate, strategy = OWNER_ONLY) Closure c) { + c.delegate = new Delegate() + c.resolveStrategy = OWNER_ONLY + c() + } +}