[GitHub] groovy pull request #797: GROOVY-8794: Add groovy-yaml subproject to support...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/797#discussion_r217889449 --- Diff: build.gradle --- @@ -183,6 +185,9 @@ dependencies { compile("org.apache.ivy:ivy:$ivyVersion") { transitive = false } +compile "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:$jacksondataformatyamlVersion" +compile "com.fasterxml.jackson.core:jackson-databind:$jacksondatabindVersion" + --- End diff -- Just curious why these 2 deps are listed here and not in `groovy-yaml/build.gradle`? ---
[GitHub] groovy pull request #797: GROOVY-8794: Add groovy-yaml subproject to support...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/797#discussion_r217889473 --- Diff: subprojects/groovy-yaml/src/main/java/groovy/yaml/YamlBuilder.java --- @@ -0,0 +1,264 @@ +/* + * 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 groovy.yaml; + +import groovy.json.JsonBuilder; +import groovy.lang.Closure; +import groovy.lang.GroovyObjectSupport; +import groovy.lang.Writable; +import groovy.yaml.util.YamlConverter; + +import java.io.IOException; +import java.io.StringReader; +import java.io.Writer; +import java.util.Collection; +import java.util.List; +import java.util.Map; + +/** + * A builder for creating YAML payloads. + * + * @since 2.5.3 + */ +public class YamlBuilder extends GroovyObjectSupport implements Writable { +private JsonBuilder jsonBuilder; + +public YamlBuilder() { +this.jsonBuilder = new JsonBuilder(); +} + +public Object getContent() { +return jsonBuilder.getContent(); +} + +/** + * Named arguments can be passed to the YAML builder instance to create a root YAML object + * + * Example: + * + * def yaml = new groovy.yaml.YamlBuilder() + * yaml name: "Guillaume", age: 33 + * + * assert yaml.toString() == '{"name":"Guillaume","age":33}' --- End diff -- Looks like most of the `YamlBuilder#toString` javadoc examples in this class shows `json` instead of `yaml` output. ---
[GitHub] groovy pull request #757: GROOVY-8008: AIOOB inner class ctor params with ru...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/757#discussion_r196172656 --- Diff: src/main/java/org/codehaus/groovy/vmplugin/v5/Java5.java --- @@ -405,6 +408,32 @@ public void configureClassNode(CompileUnit compileUnit, ClassNode classNode) { } } +/** + * Synthetic parameters such as those added for inner class constructors may not be + * included in the parameter annotations array. This is the case when at least one + * parameter of an inner class constructor is annotated with a RUNTIME retention + * policy. This method will normalize the annotation array so that it contains the + * same number of elements as the array returned from {@link Constructor#getParameterTypes()}. + * + * If adjustment is required, the adjusted array will be pre-pended will zero-length --- End diff -- Fixed, thanks. ---
[GitHub] groovy pull request #757: GROOVY-8008: AIOOB inner class ctor params with ru...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/757 GROOVY-8008: AIOOB inner class ctor params with runtime annotations Should also address [GROOVY-8505](https://issues.apache.org/jira/browse/GROOVY-8505) which is the same issue. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8008-aioob-inner-class-ctor-param-annos Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/757.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 #757 commit e5e8da40dc3e2d1d141280ee5162247da7ed3083 Author: John Wagenleitner Date: 2018-06-15T02:37:56Z GROOVY-8008: AIOOB inner class ctor params with runtime annotations ---
[GitHub] groovy pull request #756: GROOVY-8614: Invalid reference generated in InnerC...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/756 GROOVY-8614: Invalid reference generated in InnerClasses attribute fo⦠â¦r nested interface You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8614-invalid-inner-nested-interface Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/756.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 #756 commit 97c9d108de7197dcdc1aa55a8fc9af18a8880e17 Author: John Wagenleitner Date: 2018-06-15T02:31:59Z GROOVY-8614: Invalid reference generated in InnerClasses attribute for nested interface ---
[GitHub] groovy pull request #753: GROOVY-8632: Groovy 2.5.0 fails to compile Google ...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/753 GROOVY-8632: Groovy 2.5.0 fails to compile Google Java Client sample code Class files can contain INNERCLASS references to other classes inner classes whose name may be the same name as a contained inner class. By storing modifiers in a map keyed by short class name there is a possibility for the wrong modifiers to be stored. Since generated class files for inner classes contain an INNERCLASS self reference, the logic can be simplified to look for a matching name and storing those access modifiers. This eliminates the need to search the outer class for the INNERCLASS reference. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8632-decompiled-cn-nested-classes Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/753.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 #753 commit 8b0d57cfc8c600296763b31a9d9f5166b17ef4f1 Author: John Wagenleitner Date: 2018-06-09T00:00:16Z GROOVY-8632: Groovy 2.5.0 fails to compile Google Java Client sample code Class files can contain INNERCLASS references to other classes inner classes whose name may be the same name as a contained inner class. By storing modifiers in a map keyed by short class name there is a possibility for the wrong modifiers to be stored. Since generated class files for inner classes contain an INNERCLASS self reference, the logic can be simplified to look for a matching name and storing those access modifiers. This eliminates the need to search the outer class for the INNERCLASS reference. ---
[GitHub] groovy pull request #747: GROOVY-8583: Fail to infer auto-return type from t...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/747 GROOVY-8583: Fail to infer auto-return type from ternary operator You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8583-stc-ternary Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/747.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 #747 commit 884ddeb4d9aca6c4a68f22f7fbd7ce2ab77f51f1 Author: John Wagenleitner Date: 2018-06-03T02:17:08Z GROOVY-8583: Fail to infer auto-return type from ternary operator ---
[GitHub] groovy pull request #729: GROOVY-8610: STC NPE using DGM collect on Iterator
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/729 GROOVY-8610: STC NPE using DGM collect on Iterator You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8610-stc-collect-iterator Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/729.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 #729 commit 137f87a3809e3d8d8f572b07f464237f3277b0b2 Author: John Wagenleitner Date: 2018-05-27T19:10:59Z GROOVY-8610: STC NPE using DGM collect on Iterator ---
[GitHub] groovy pull request #724: Release 2.5.0 related fixes
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/724 Release 2.5.0 related fixes Some recent commits seem to cause problems with the Nextflow 2.5.0 snapshot builds on the CI server. One can lead to a NPE (commit 90486ae1075d14a62d14452abfea0c27485a67b5) because fieldNode can be reassigned `null` after the `instanceof` check. Another change (commit 57cfd2a3e4d985b3248569ea1d92b02c59627703) seems to have caused an issue with assigning parameterized types. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy rel25fixes Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/724.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 #724 commit 1376361bb9089b1d54e9938a7055610b67b8df0e Author: John Wagenleitner Date: 2018-05-24T21:59:20Z Fix parameter matching for parameterized types Revert of change 57cfd2a3e4d985b3 and fixes issues with Nextflow CI builds. commit c7d3e6d8916f78544e7354837fa0ff4b811931f6 Author: John Wagenleitner Date: 2018-05-25T20:16:18Z Fix NPE if accessed property not a member of the owning class ---
[GitHub] groovy pull request #:
Github user jwagenleitner commented on the pull request: https://github.com/apache/groovy/commit/869c365161457c050d5a54c7ff43d73d3263f34e#commitcomment-29066081 thanks! ---
[GitHub] groovy pull request #713: GROOVY-8590: STC incorrectly infers type of nested...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/713 GROOVY-8590: STC incorrectly infers type of nested method call used i⦠â¦n a return stmt You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8590-STC-nested-mc-return Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/713.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 #713 commit 69315c3252a77fa325a14c901b5e8edf01bfe2db Author: John Wagenleitner Date: 2018-05-20T05:32:20Z GROOVY-8590: STC incorrectly infers type of nested method call used in a return stmt ---
[GitHub] groovy pull request #712: GROOVY-8171: Escaped dollar slashy difference betw...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/712 GROOVY-8171: Escaped dollar slashy difference between old and parrot ⦠â¦parsers You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8171-dollar-slash-parrot Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/712.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 #712 commit ae89f1b96d6fcf60837b995d871e3489312adb87 Author: John Wagenleitner Date: 2018-05-19T15:45:25Z GROOVY-8171: Escaped dollar slashy difference between old and parrot parsers ---
[GitHub] groovy pull request #707: GROOVY-8509: SC error call to protected method fro...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/707 GROOVY-8509: SC error call to protected method from same package Because they were closely related, this PR also includes the following: 1. A change to the fix introduced by GROOVY-7883 (b1d1232770aa) to prevent filtering protected methods in the `StaticTypeCheckingVisitor` if caller is in the same package. From the same commit, removes the test `Groovy7883Bug#test3` because it should compile and the new test introduced for GROOVY-8509 in `MethodCallsStaticCompilation` takes it place. 1. A change to revert groovy.sql.Sql#asList (b1d1232770aa) back to `protected`. Fix for GROOVY-7883 enforced visibility for method calls in `TypedChecked` and `CompileStatic` modes. Test was originally put in place to verify the added ClosureParams, so changed test to access method via subclass so that the access modifier for the method can remain protected. I think the spirit of the test is preserved in this case without having to open access to the method under test. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8509-SC-protected-same-package Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/707.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 #707 commit c76783966f4f9198498abdf6f4696dc36e449cd5 Author: John Wagenleitner Date: 2018-05-16T14:58:23Z Revert change to groovy.sql.Sql#asList in commit b1d1232770aa Fix for GROOVY-7883 enforced visibility for method calls in TypedChecked and CompileStatic modes. Test was originally put in place to verify the added ClosureParams, so changed test to access method via subclass so that the access modifier for the method can remain protected. commit 8821e8406adeec4d69894b739a2d941c51266c87 Author: John Wagenleitner Date: 2018-05-16T15:34:48Z GROOVY-8509: SC error call to protected method from same package Also includes a change to the fix introduced by GROOVY-7883 (b1d1232770aa) to prevent filtering protected methods in the StaticTypeCheckingVisitor if caller is in the same package. From the same commit, removes the test Groovy7883Bug#test3 because it should compile and the new test introduced for GROOVY-8509 in MethodCallsStaticCompilation takes it place. ---
[GitHub] groovy pull request #:
Github user jwagenleitner commented on the pull request: https://github.com/apache/groovy/commit/b1d1232770aade9672668df4dbc6aa2e2076fa9e#commitcomment-28988363 In subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java: In subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java on line 3976: I think the method access should not be opened to satisfy a test. Usually `@TypeChecked` works similar to dynamic mode for method dispatch so accessing a protected method would normally be allowed. If `@TypeChecked` is changed to enforce visibility rules then maybe better to alter the test to use a sub-class so as to not have to change the access modifier here? ---
[GitHub] groovy pull request #671: GROOVY-8422: Incorrect properties copy in Sql.newI...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/671 GROOVY-8422: Incorrect properties copy in Sql.newInstance The provided Properties should be passed to the DriverManager as-is. A copy is only needed when changes are made to the provided Properties in order to mask sensitive information for logging purposes. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8422-sql-props Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/671.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 #671 commit 1f833b4af4e5e5ac667967b55f279e9534d5ec58 Author: John Wagenleitner Date: 2018-03-05T16:35:33Z GROOVY-8422: Incorrect properties copy in Sql.newInstance The provided Properties should be passed to the DriverManager as-is. A copy is only needed when changes are made to the provided Properties in order to mask sensitive information for logging purposes. ---
[GitHub] groovy pull request #670: GROOVY-8475: unable to instantiate objects using t...
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/670 ---
[GitHub] groovy pull request #670: GROOVY-8475: unable to instantiate objects using t...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/670 GROOVY-8475: unable to instantiate objects using the "new" keyword in groovysh PR #100 was merged into 2_5_X and comments on GROOVY-7562 indicate it wasn't merged into 2_4_X due to binary compatibility concerns. I think this backport addresses those concerns and this PR is targeted only for the 2_4_X branch. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8475-groovysh-new Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/670.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 #670 commit c220753e867f733ac7c68a56d5a8dd9aa5ac60dc Author: John Wagenleitner Date: 2018-03-04T01:53:40Z GROOVY-8475: unable to instantiate objects using the "new" keyword in groovysh Backport fix "GROOVY-7562 Groovysh: Fix custom class instantiation impossible with Interpreter Mode" for the 2_4_X branch. Retain binary compatibilty by retaining and deprecating methods removed in the original fix that was applied to 2_5_X. commit 567bd1e525b8752a7594bf594e86596c9e3f19bb Author: John Wagenleitner Date: 2018-03-04T04:21:16Z backport fix GROOVY-7562 for the 2_4_X branch Groovysh: Fix imports not working at all in interpreter mode ---
[GitHub] groovy pull request #563: vmplugin
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/563 ---
[GitHub] groovy pull request #610: GROOVY-8326: @Override should not copied onto meth...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/610 GROOVY-8326: @Override should not copied onto methods generated by ap⦠â¦plying @Memoize You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8326-Memoized-Override Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/610.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 #610 commit 8bb23c4a5574925a1eecfe57b1a8acbcafbbcb5a Author: John Wagenleitner Date: 2017-10-01T04:06:49Z GROOVY-8326: @Override should not copied onto methods generated by applying @Memoize ---
[GitHub] groovy pull request #609: GROOVY-8213: Closures are maybe not Threadsafe
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/609 GROOVY-8213: Closures are maybe not Threadsafe To address @blackdrag 's comment in the JIRA I created a JMH benchmark to test a volatile read vs a recheck and sync, https://github.com/jwagenleitner/testing-groovy/tree/master/issues/groovy8213. Based on my runs the volatile read is very close to a normal read. Doing a branched check (without needing to sync) was actually worse in terms of performance. It seems with the `--parallel` feature in Gradle and builds being run on large many core servers this problem seems to be coming up more frequently. See https://github.com/gradle/gradle/issues/1420. While I do believe the publication of the `initialized` variable is an issue and should be addressed, another possible cause of this sort of problem could be related to the way the [`respondTo` methods are implemented by unsetting the `initialized` variable during a call to `super.initialize()` in the `loadMetaInfo() ` method](https://github.com/apache/groovy/blob/7b101dd98ffc04b1bfd2447bbe277340d8954add/src/main/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java#L724-L740). But in this case and the referenced Gradle issue I'm not sure the `respondTo` methods are in play. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8213-Closure-Init-Threadsafe Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/609.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 #609 commit 01946dbf2bdd11bb1fa402131ba50fef26f889eb Author: John Wagenleitner Date: 2017-10-01T02:31:16Z GROOVY-8213: Closures are maybe not Threadsafe ---
[GitHub] groovy pull request #605: GROOVY-8220: SC GroovyCastException on parameter f...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/605 GROOVY-8220: SC GroovyCastException on parameter flow typing GROOVY-8157 introduced flow typing for parameters and this fix is required in order to track their assignments in `if` branches for temporary type assignments. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8220-ParameterFlowTyping Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/605.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 #605 commit ad5074b4db073ebefd1af21676675671a62f6064 Author: John Wagenleitner Date: 2017-09-08T00:27:06Z GROOVY-8220: SC GroovyCastException on parameter flow typing GROOVY-8157 introduced flow typing for parameters and this fix is required in order to track their assignments in `if` branches for temporary type assignments. ---
[GitHub] groovy pull request #593: GROOVY-8303: VerifyError for nested class this cal...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/593 GROOVY-8303: VerifyError for nested class this call to static method You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8303-VerifyError-Nested Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/593.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 #593 commit 29614bfa2ef1a3c57412b8148125d43b1911dc43 Author: John Wagenleitner Date: 2017-08-27T21:23:29Z GROOVY-8303: VerifyError for nested class this call to static method --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #592: GROOVY-8205: Regression test for STC Enum values D...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/592 GROOVY-8205: Regression test for STC Enum values DGM methods Issue fixed by commit 8c218dec34 (GROOVY-7283) You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8205-EachEnumValuesTest Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/592.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 #592 commit 95674471a8070b812966f3d50315d7e4f4e19358 Author: John Wagenleitner Date: 2017-08-26T18:55:25Z GROOVY-8205: Regression test for STC Enum values DGM methods Issue fixed by commit 8c218dec34 (GROOVY-7283) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #584: GROOVY-8249: Newify local variable declaration fai...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/584 GROOVY-8249: Newify local variable declaration fails to resolve class⦠⦠expression You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8249-Newify-VarDecl-ClassResolve Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/584.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 #584 commit 2c11c1b72ff47e6c5ccb2134a7ea86da8cabc028 Author: John Wagenleitner Date: 2017-08-14T00:15:00Z GROOVY-8249: Newify local variable declaration fails to resolve class expression --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #583: migrate benchmarks to JMH
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/583 migrate benchmarks to JMH You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy bench2perf Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/583.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 #583 commit d561bbbe1ef8ff952b9d1297562824e68aca735b Author: John Wagenleitner Date: 2017-07-23T19:54:48Z migrate benchmarks to JMH --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #582: GROOVY-8269: Unclear definition of default behavio...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/582 GROOVY-8269: Unclear definition of default behavior for trait multipl⦠â¦e inheritance conflicts You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8269-trait-conflict-doc Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/582.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 #582 commit ca3a10c1e22be899bcd8534a7aee4f5bdaf197d2 Author: John Wagenleitner Date: 2017-08-13T19:07:33Z GROOVY-8269: Unclear definition of default behavior for trait multiple inheritance conflicts --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #581: GROOVY-8208: VariableExpressionTransformer does no...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/581 GROOVY-8208: VariableExpressionTransformer does not set source positi⦠â¦on on property expressions You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8208-varxform Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/581.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 #581 commit 48ce82bce883ee8bcba777a831ad1ee98b70bf45 Author: John Wagenleitner Date: 2017-08-13T04:43:06Z GROOVY-8208: VariableExpressionTransformer does not set source position on property expressions --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #569: include indy in benchmark runs
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/569 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #579: GROOVY-8242: @Newify default attribute value
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/579 GROOVY-8242: @Newify default attribute value Class values are only required for Python-style conversions so the attribute should default to an empty array to indicate it is not strictly required. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8242-Newify-value-default Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/579.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 #579 commit e68a7c7ca60e9c6453565bbb91720dfe6479567c Author: John Wagenleitner Date: 2017-08-05T18:05:17Z GROOVY-8242: @Newify default attribute value Class values are only required for Python-style conversions so the attribute should default to an empty array to indicate it is not strictly required. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #576: GROOVY-7995: @CS closure call from closure
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/576 GROOVY-7995: @CS closure call from closure Short syntax of closure call invokes wrong closure if wrapped in another closure. This fix includes a combination of the contributed commit from PR #460 along with the patch (see PR comments) provided by Jochen. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy pr460 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/576.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 #576 commit 8916d43f298d26814c664555cae1a800e2cbfe5b Author: John Wagenleitner Date: 2017-07-22T17:06:21Z GROOVY-7995: @CS closure call from closure Short syntax of closure call invokes wrong closure if wrapped in another closure. This fix includes a combination of the contributed commit from PR #460 along with the patch (see PR comments) provided by Jochen. Thanks for @blindpirate for the contribution. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #575: cache GroovyRunnerRegistry values
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/575 cache GroovyRunnerRegistry values The registry should be read heavy and most use made of the iterator. Few writes/loads should occur, so values should be cached in order to optimize iteration. Benchmarked the change against using a read lock but `volatile` was significantly faster. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy runner-values Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/575.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 #575 commit 28576c8da816a970d3a1151744f6b073f374e9a4 Author: John Wagenleitner Date: 2017-07-15T14:00:29Z cache GroovyRunnerRegistry values The registry should be read heavy and most use made of the iterator. Few writes/loads should occur, so values should be cached in order to optimize iteration. commit 43b26fd640a1867c557681d4381113a974cce680 Author: John Wagenleitner Date: 2017-07-17T01:33:20Z GroovyRunnerRegistry iterator benchmarks --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #573: add JMH to performance subproject
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/573 add JMH to performance subproject You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy jmh-perf Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/573.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 #573 commit d1d137ea0191cc9265c1ae444ef58d89a1e876da Author: John Wagenleitner Date: 2017-07-09T18:49:55Z add JMH to performance subproject --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #572: GROOVY-8251: Implement withCloseable on AutoClosea...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/572 GROOVY-8251: Implement withCloseable on AutoCloseable `NioGroovyMethods.withAutoCloseable` was added for 2.5.0 as part of [GROOVY-7572](https://issues.apache.org/jira/browse/GROOVY-7572). It appears it was originally placed here because at the time Java 6 was still used, but now 2.5.0 is 7+. First commit proposes moving the method (as-is) into the core `IOGroovyMethods` class. This is where the `withCloseable(java.io.Closeable...)` method is, so also part of this change is moving the tests for it from NIO to core. Since 2.5.0 is not been officially released I don't think there is a need to deprecate or worry about binary compatibility for the `withAutoCloseable` method. Second commit proposes to rename the new but unreleased method from `withAutoCloseable` to `withCloseable` to match the existing method on `java.io.Closeable`. See discussion on [GROOVY-8251](https://issues.apache.org/jira/browse/GROOVY-8251). You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8251-AutoCloseable Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/572.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 #572 commit 912200153193849fc35a09a053898d46c7f1dacd Author: John Wagenleitner Date: 2017-07-09T00:22:36Z GROOVY-8251: Implement withCloseable on AutoCloseable Relocate withAutoCloseable from NIO subproject to core. Since AutoCloseable is not an NIO related class and release 2.5.0 will target Java 7, the method should be available as part of core. Relocated withCloseable tests to core since that method is deprecated in the NIO module. commit 261bc174980da8d6b6f4cdb54b8f0ab98c64c4fd Author: John Wagenleitner Date: 2017-07-09T01:13:48Z GROOVY-8251: rename withAutoCloseable to withCloseable --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #559: GROOVY-8222: Setting Source Position in newly crea...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/559#discussion_r126288792 --- Diff: src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java --- @@ -1218,7 +1218,7 @@ public void visitVariableExpression(VariableExpression expression) { BytecodeVariable variable = controller.getCompileStack().getVariable(variableName, false); if (variable == null) { -processClassVariable(variableName); +processClassVariable(expression); --- End diff -- Thanks for the PR. I merged before seeing the comment from @blackdrag, so another refactor PR would be good. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #559: GROOVY-8222: Setting Source Position in newly crea...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/559#discussion_r126286817 --- Diff: src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java --- @@ -1251,11 +1251,13 @@ private void processClassVariable(String name) { mv.visitInsn(DUP); loadThisOrOwner(); -mv.visitLdcInsn(name); +mv.visitLdcInsn(expression.getName()); mv.visitMethodInsn(INVOKESPECIAL, "org/codehaus/groovy/runtime/ScriptReference", "", "(Lgroovy/lang/Script;Ljava/lang/String;)V", false); } else { -PropertyExpression pexp = new PropertyExpression(new VariableExpression("this"), name); +PropertyExpression pexp = new PropertyExpression(new VariableExpression("this"), expression.getName()); --- End diff -- Scratch that comment, I was looking at the wrong source. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #559: GROOVY-8222: Setting Source Position in newly crea...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/559#discussion_r126286728 --- Diff: src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java --- @@ -1251,11 +1251,13 @@ private void processClassVariable(String name) { mv.visitInsn(DUP); loadThisOrOwner(); -mv.visitLdcInsn(name); +mv.visitLdcInsn(expression.getName()); mv.visitMethodInsn(INVOKESPECIAL, "org/codehaus/groovy/runtime/ScriptReference", "", "(Lgroovy/lang/Script;Ljava/lang/String;)V", false); } else { -PropertyExpression pexp = new PropertyExpression(new VariableExpression("this"), name); +PropertyExpression pexp = new PropertyExpression(new VariableExpression("this"), expression.getName()); --- End diff -- The patch provided in the JIRA creates a new `VariableExpression` (see below), I think that needs to be done so the source position is not set on the constant `VariableExpress.THIS_EXPRESSION` via the `getObjectExpression().setSourcePosition(..)` call. ```java PropertyExpression pexp = new PropertyExpression(new VariableExpression("this", ClassHelper.DYNAMIC_TYPE), name); ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #558: GROOVY-8218 @Sortable allows reversed natural orde...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/558#discussion_r125553663 --- Diff: src/main/groovy/transform/Sortable.java --- @@ -145,4 +170,9 @@ * Must not be used if 'includes' is used. */ String[] excludes() default {}; + +/** + * Set to true so that comparator uses reversed natural order. + */ --- End diff -- Would be good to add `@since 2.5.0` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #558: GROOVY-8218 @Sortable allows reversed natural orde...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/558#discussion_r12008 --- Diff: src/main/org/codehaus/groovy/transform/SortableASTTransformation.java --- @@ -82,6 +82,7 @@ public void visit(ASTNode[] nodes, SourceUnit source) { private void createSortable(AnnotationNode annotation, ClassNode classNode) { List includes = getMemberStringList(annotation, "includes"); List excludes = getMemberStringList(annotation, "excludes"); +boolean reversed = getMemberBoolValue(annotation, "reversed"); --- End diff -- `boolean reversed = memberHasValue(annotation, "reversed", true);` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #558: GROOVY-8218 @Sortable allows reversed natural orde...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/558#discussion_r125554738 --- Diff: src/main/org/codehaus/groovy/transform/AbstractASTTransformation.java --- @@ -115,6 +115,14 @@ public int getMemberIntValue(AnnotationNode node, String name) { return 0; } +public boolean getMemberBoolValue(AnnotationNode node, String name) { --- End diff -- I think it's common to use `memberHasValue` for evaluating boolean attributes, so this new method is probably not needed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #558: GROOVY-8218 @Sortable allows reversed natural orde...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/558#discussion_r125553175 --- Diff: src/main/org/codehaus/groovy/transform/SortableASTTransformation.java --- @@ -112,7 +113,7 @@ private static void implementComparable(ClassNode classNode) { } } -private static Statement createCompareToMethodBody(List properties) { +private static Statement createCompareToMethodBody(List properties, Boolean reversed) { --- End diff -- `boolean` as the parameter type would be consistent with the other methods below. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #558: GROOVY-8218 @Sortable allows reversed natural orde...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/558#discussion_r125552693 --- Diff: src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java --- @@ -219,10 +219,38 @@ public static ClosureExpression closureX(Statement code) { return result; } +/** + * Build a binary expression that compares two values + * @param lhv expression for the value to compare from + * @param rhv expression for the value value to compare to + * @return the expression comparing two values + */ public static BinaryExpression cmpX(Expression lhv, Expression rhv) { return new BinaryExpression(lhv, CMP, rhv); } +/** + * Build a binary expression that compares two values + * @param lhv expression for the value to compare from + * @param rhv expression for the value value to compare to + * @param reversed whether to use natural ordering or reversed natural ordering + * @return the expression comparing two values + */ +public static BinaryExpression cmpX(Expression lhv, Expression rhv, boolean reversed) { +return (reversed) ? cmpXReversed(lhv, rhv) : cmpX(lhv, rhv); --- End diff -- What about `return (reversed) ? cmpX(rhv, lhv) : cmpX(lhv, rhv);`? Could eliminate the need for the new `cmpXReversed` method. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #571: GROOVY-8245: @Newify(auto=false) not transforming ...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/571 GROOVY-8245: @Newify(auto=false) not transforming declarations Also added default for `value` attribute for [GROOVY-8242](https://issues.apache.org/jira/browse/GROOVY-8242). You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8245-Newify Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/571.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 #571 commit f2c80cf2403e40b7c7f22eaa778bec35a8c80bee Author: John Wagenleitner Date: 2017-07-04T16:37:04Z GROOVY-8245: @Newify(auto=false) not transforming declarations --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #569: include indy in benchmark runs
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/569 include indy in benchmark runs You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy benchmarks Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/569.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 #569 commit ba98d61628f4b2d284e4edaf3053a7adc347bd29 Author: John Wagenleitner Date: 2017-06-28T00:45:23Z include indy in benchmark runs --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #564: GROOVY-8197: Make JUnit3/4 GroovyRunners
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/564 GROOVY-8197: Make JUnit3/4 GroovyRunners You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8197-groovyrunners Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/564.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 #564 commit 469f0e970999e84d1e878801ac56b22af73b9cbf Author: John Wagenleitner Date: 2017-06-09T22:41:38Z GROOVY-8197: Make JUnit3/4 GroovyRunners --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #563: vmplugin
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/563 vmplugin Alternative to PR #562. I tried to make incremental commits to make it easier to review. Still outstanding are the indy related classes in `org.codehaus.groovy.vmplugin.v7`. Just not sure what a good package name for those would be, maybe `org.apache.groovy.internal.runtime.indy`? You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy vmplugin25 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/563.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 #563 commit 0ee306754fa0dcfd187999228c4a414bd5db3457 Author: John Wagenleitner Date: 2017-06-20T23:26:33Z move Java8 plugin to new org.apache.groovy.internal.vmplugin package commit 26e0d492329a6081ead13a3cd943fd1a7a3a8703 Author: John Wagenleitner Date: 2017-06-20T23:27:20Z move core VMPlugin classes to new package commit d07feef77ea29fc0de5a2b6537a67f9f7302712b Author: John Wagenleitner Date: 2017-06-20T23:27:43Z extract Java5 plugin into VMPluginBase class commit 9329434f22ff4ff4569535cf882a2372fc366f54 Author: John Wagenleitner Date: 2017-06-20T23:28:31Z use new VMPluginFactory commit 55dbde979a1a7ef83979e535812517d08896b4dc Author: John Wagenleitner Date: 2017-06-20T23:28:47Z deprecate Java6 plugin commit a80fea8d07f8c722de2c37b593b3d72c16b02b6f Author: John Wagenleitner Date: 2017-06-20T23:29:04Z Java7 extends VMPluginBase commit 0513559f52a33584018a85834b8158d781d12a29 Author: John Wagenleitner Date: 2017-06-20T23:29:29Z deprecate Java5 plugin default methods commit 8072c5f64e09378d571f8a565507a148f2b3e536 Author: John Wagenleitner Date: 2017-06-20T23:30:04Z deprecate Java5 JUnit4Utils commit 5fdc11f8339171054b060de27275130e47474eaa Author: John Wagenleitner Date: 2017-06-20T23:30:15Z move Java7 VM plugin to new package commit f9517054093d064c8a59e1b67ad63ac4e461c409 Author: John Wagenleitner Date: 2017-06-20T23:30:37Z extract Java7 methods into base --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #562: rename Java8 package from vm8 to v8 for consistenc...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/562 rename Java8 package from vm8 to v8 for consistency You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy v8 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/562.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 #562 commit f963d8fa08d8d61ac5e9c01e809b617fe45931c9 Author: John Wagenleitner Date: 2017-06-19T01:06:29Z rename Java8 package from vm8 to v8 for consistency --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #561: GROOVY-8230: Deadlock in GroovyClassLoader
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/561 GROOVY-8230: Deadlock in GroovyClassLoader You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8230-gcl-deadlock Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/561.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 #561 commit cc2ab1a5fe09d56de905d0b0a3b3a87fcf0d6a60 Author: John Wagenleitner Date: 2017-06-19T00:16:07Z GROOVY-8230: Deadlock in GroovyClassLoader --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #560: GROOVY-8226: JSR308 initial plumbing tweaks
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/560#discussion_r121416370 --- Diff: src/main/org/codehaus/groovy/vmplugin/vm8/Java8.java --- @@ -51,4 +53,13 @@ public int getVersion() { return 8; } +protected int getElementCode(ElementType value) { --- End diff -- might be good to have `@Override` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #557: GROOVY-8056: GroovyCodeSource(URL) can leak a file...
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/557 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #500: GROOVY-8056: GroovyCodeSource(URL) can leak a file...
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/500 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #557: GROOVY-8056: GroovyCodeSource(URL) can leak a file...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/557 GROOVY-8056: GroovyCodeSource(URL) can leak a file handler A safer fix in terms of compatibility compared to PR #500. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8056-urlcon-leak Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/557.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 #557 commit d99ff70a729448d75d3ca5b98c70733ba1ca428a Author: John Wagenleitner Date: 2017-06-03T14:50:41Z GROOVY-8056: GroovyCodeSource(URL) can leak a file handler --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #553: require JDK 8+ for release builds (2_5_X/2_6_X)
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/553 require JDK 8+ for release builds (2_5_X/2_6_X) For 2_5_X and 2_6_X in order to compile all features JDK 8+ is required for building releases (related to PR #545). Also included a commit to add JSR223 DGMs in the GDK docs. These methods were moved from `org.codehaus.groovy.vmplugin.v6.PluginDefaultGroovyMethods` and `org.codehaus.groovy.vmplugin.v6.PluginStaticGroovyMethods` but the docs build was not updated to include the new files. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 7611-build-25x Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/553.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 #553 commit 2a2de466152437e0ca97dfd70e3aaf0cae03d3f6 Author: John Wagenleitner Date: 2017-05-27T22:21:43Z require JDK 8+ for release builds Related to change for GROOVY-7611 (PR #545), JDK 8+ is required in order to compile all features. commit 5764f17eeb5f6b5b33b27480dc09294db487368d Author: John Wagenleitner Date: 2017-05-27T22:23:58Z include JSR223 DGMs in generated GDK docs --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #551: GROOVY-8204: @Delegate on arrays causes NPE during...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/551#discussion_r118507014 --- Diff: src/main/org/codehaus/groovy/ast/ClassHelper.java --- @@ -440,12 +440,15 @@ private static boolean hasUsableImplementation(ClassNode c, MethodNode m) { /** * Returns a super class or interface for a given class depending on a given target. * If the target is no super class or interface, then null will be returned. + * For a non=primitive array type, returns an array of the componentType's superclass. --- End diff -- non-primitive --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #545: GROOVY-7611: java.util.Optional should evaluate to...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/545 GROOVY-7611: java.util.Optional should evaluate to false if empty (Java8 VMPlugin) Target for this would be `2_5_X` and above. For `2_5_X` and `2_6_X` it would require that the release process build with JDK 8 so would need to backport some of the build checks for Java version to those branches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 7611-Optional Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/545.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 #545 commit 720dced06b1ec447d77b00550b8b299dfe3e18d9 Author: John Wagenleitner Date: 2017-05-21T17:16:06Z GROOVY-7611: java.util.Optional should evaluate to false if empty --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #544: refactor: type safety and formatting
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/544#discussion_r117615134 --- Diff: src/main/org/codehaus/groovy/transform/trait/Traits.java --- @@ -355,19 +355,19 @@ static String getSuperTraitMethodName(ClassNode trait, String method) { */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) -public static @interface Implemented {} +public @interface Implemented {} /** * Internal annotation used to indicate that a method is a bridge method to a trait * default implementation. */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) - public static @interface TraitBridge { --- End diff -- The `static` modifier is redundant on inner interfaces. The bytecode will remain unchanged with or without the modifier so the code referenced should continue to work. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #544: refactor: type safety and formatting
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/544 refactor: type safety and formatting You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy refactor Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/544.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 #544 commit 71da9595ff1b3cda9534208620025efd739a1455 Author: John Wagenleitner Date: 2017-05-20T16:00:23Z refactor: type safety commit 5f2642b09f2625177b962ce4faa805e86229185d Author: John Wagenleitner Date: 2017-05-20T16:35:40Z refactor: formatting and type safety --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #:
Github user jwagenleitner commented on the pull request: https://github.com/apache/groovy/commit/b02d2f57c09ba6ba46553ed77aff191ec9ab274e#commitcomment-22218912 In src/main/org/apache/groovy/util/Maps.java: In src/main/org/apache/groovy/util/Maps.java on line 11: JDK9 added `Map.ofEntries` to handle this and also added `Map.of` which is defined for up to 10 elements and both are type safe. All uses of this will not be type safe. Even though this is used internally, it would be nice to have a type safe way to build maps and not use raw types. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #543: GROOVY-8166: Repeated operations in AnnotationColl...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/543 GROOVY-8166: Repeated operations in AnnotationCollectorTransform and ⦠â¦Traits You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 8166 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/543.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 #543 commit 72c01601d4cdd00622e6215b2a8d8dcac2ca4a89 Author: John Wagenleitner Date: 2017-05-19T19:48:53Z GROOVY-8166: Repeated operations in AnnotationCollectorTransform and Traits --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/532#discussion_r117078128 --- Diff: src/main/groovy/lang/MetaClassImpl.java --- @@ -1832,6 +1832,9 @@ public Object getProperty(Class sender, Object object, String name, boolean useS } catch (IllegalArgumentException e) { // can't access the field directly but there may be a getter mp = null; +} catch (GroovyRuntimeException e) { +// can't access the field directly but there may be a getter +mp = null; --- End diff -- I don't understand why this was added. Was there a test that failed because of the other changes that required this. I would have assumed an `AccessControlException` from `CachedField.getProperty` would be treated similar to the `IllegalAccessException` in the same method. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #533: GROOVY-7840: Verifier#makeDescriptorWithoutReturnT...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/533#discussion_r116533820 --- Diff: src/main/org/apache/groovy/ast/tools/MethodNodeUtils.java --- @@ -0,0 +1,66 @@ +/* + * 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 org.apache.groovy.ast.tools; + +import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.Parameter; + +public class MethodNodeUtils { +/** + * Return the method node's descriptor including its + * name and parameter types without generics. + * + * @param mNode the method node + * @return the method node's abbreviated descriptor excluding the return type + */ +public static String methodDescriptorWithoutReturnType(MethodNode mNode) { --- End diff -- That is what is sort of confusing, the method descriptor for bytecode vs the method signature used to detect duplicate signatures. Agree that the formatting is particular to it's use in the `Verifier` and having in a separate utils class is probably the best approach. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #540: GROOVY-7535: Groovy category throwing MissingMetho...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/540 GROOVY-7535: Groovy category throwing MissingMethodException and Miss⦠â¦ingPropertyException when using multiple threads You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy7535 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/540.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 #540 commit 1c2ca41491fd919ddc1c543ec92a8ab7ca3b860b Author: John Wagenleitner Date: 2017-05-14T01:51:47Z GROOVY-7535: Groovy category throwing MissingMethodException and MissingPropertyException when using multiple threads --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/532#discussion_r116365638 --- Diff: src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java --- @@ -0,0 +1,62 @@ +/* + * 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 org.codehaus.groovy.reflection; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.ReflectPermission; + +import groovy.lang.GroovyObject; + +class AccessPermissionChecker { --- End diff -- Might be good to add a private constructor to signal that no instances of this method are desired, since it only contains static methods. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/532#discussion_r116365514 --- Diff: src/main/org/codehaus/groovy/reflection/CachedMethod.java --- @@ -324,6 +337,12 @@ else if (o2 instanceof CachedMethod) } public Method getCachedMethod() { +try { +AccessPermissionChecker.checkAccessPermission(cachedMethod); +} +catch (AccessControlException ex) { +throw new IllegalArgumentException("Illegal access to method" + cachedMethod.getName()); --- End diff -- see comment on `setAccessible` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/532#discussion_r116365223 --- Diff: src/main/org/codehaus/groovy/reflection/CachedField.java --- @@ -65,6 +72,12 @@ public Object getProperty(final Object object) { * @throws RuntimeException if the property could not be set */ public void setProperty(final Object object, Object newValue) { +try { +AccessPermissionChecker.checkAccessPermission(field); +} +catch (AccessControlException ex) { +throw new IllegalArgumentException("Illegal access to field" + " " + field.getName()); --- End diff -- see above comment about `GroovyRuntimeException` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/532#discussion_r116365151 --- Diff: src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java --- @@ -0,0 +1,62 @@ +/* + * 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 org.codehaus.groovy.reflection; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.ReflectPermission; + +import groovy.lang.GroovyObject; + +class AccessPermissionChecker { + + private static final ReflectPermission REFLECT_PERMISSION = new ReflectPermission("suppressAccessChecks"); + + static private void checkAccessPermission(Class declaringClass, final int modifiers, boolean isAccessible) { + final SecurityManager securityManager = System.getSecurityManager(); + if (isAccessible && securityManager != null) { + if ((modifiers & Modifier.PRIVATE) != 0 + || ((modifiers & (Modifier.PUBLIC | Modifier.PROTECTED)) == 0 +&& packageCanNotBeAddedAnotherClass(declaringClass)) + && !GroovyObject.class.isAssignableFrom(declaringClass)) { + securityManager.checkPermission(REFLECT_PERMISSION); +} +else if ((modifiers & (Modifier.PROTECTED)) != 0 + && declaringClass.equals(ClassLoader.class)){ + securityManager.checkCreateClassLoader(); + } + } + } + + private static boolean packageCanNotBeAddedAnotherClass(Class declaringClass) { + return declaringClass.getName().startsWith("java."); + } + + static public void checkAccessPermission(Method method) { + checkAccessPermission(method.getDeclaringClass(), method.getModifiers(), method.isAccessible() + ); + } + + public static void checkAccessPermission(Field field) { --- End diff -- `public` isn't needed --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/532#discussion_r116365503 --- Diff: src/main/org/codehaus/groovy/reflection/CachedMethod.java --- @@ -124,6 +131,12 @@ public String getSignature() { } public final Method setAccessible() { +try { +AccessPermissionChecker.checkAccessPermission(cachedMethod); +} +catch (AccessControlException ex) { +throw new IllegalArgumentException("Illegal access to method" + cachedMethod.getName()); --- End diff -- No arguments to this method so `IllegalArgumentException` doesn't seem right. Think it might be good to either just remove the `try/catch` or wrap in a `GroovyRuntimeException` (to capture cachedMethod.getName). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/532#discussion_r116365354 --- Diff: src/main/org/codehaus/groovy/reflection/CachedMethod.java --- @@ -90,6 +91,12 @@ public CachedClass getDeclaringClass() { public final Object invoke(Object object, Object[] arguments) { try { +AccessPermissionChecker.checkAccessPermission(cachedMethod); +} +catch (AccessControlException ex) { +throw new InvokerInvocationException(new IllegalArgumentException("Illegal access to method" + cachedMethod.getName())); --- End diff -- Throwing an `IllegalArgumentException` could be confusing since it's not the `arguments` passed to the invoked method causing the problem. Maybe `throw new InvokerInvocationException(ex)` or ``` throw new InvokerInvocationException( new AccessControlException("Illegal access to method" + cachedMethod.getName(), ex) ) ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/532#discussion_r116365210 --- Diff: src/main/org/codehaus/groovy/reflection/CachedField.java --- @@ -51,6 +52,12 @@ public int getModifiers() { */ public Object getProperty(final Object object) { try { +AccessPermissionChecker.checkAccessPermission(field); +} +catch (AccessControlException ex) { +throw new IllegalArgumentException("Illegal access to field" + " " + field.getName()); --- End diff -- Any reason for using `IllegalArgumentException`? I think the following may be more appropriate: ```java throw new GroovyRuntimeException("Illegal access to field " + field.getName() + ".", ex); ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/532#discussion_r116365112 --- Diff: src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java --- @@ -0,0 +1,62 @@ +/* + * 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 org.codehaus.groovy.reflection; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.ReflectPermission; + +import groovy.lang.GroovyObject; + +class AccessPermissionChecker { + + private static final ReflectPermission REFLECT_PERMISSION = new ReflectPermission("suppressAccessChecks"); + + static private void checkAccessPermission(Class declaringClass, final int modifiers, boolean isAccessible) { + final SecurityManager securityManager = System.getSecurityManager(); --- End diff -- Might be good to exit early if `securityManager == null`, then wont have to check again below. Also, be good to change order to be `private static void ...`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #532: Prevent CachedField and CachedMethod from leaking ...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/532#discussion_r116365147 --- Diff: src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java --- @@ -0,0 +1,62 @@ +/* + * 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 org.codehaus.groovy.reflection; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.ReflectPermission; + +import groovy.lang.GroovyObject; + +class AccessPermissionChecker { + + private static final ReflectPermission REFLECT_PERMISSION = new ReflectPermission("suppressAccessChecks"); + + static private void checkAccessPermission(Class declaringClass, final int modifiers, boolean isAccessible) { + final SecurityManager securityManager = System.getSecurityManager(); + if (isAccessible && securityManager != null) { + if ((modifiers & Modifier.PRIVATE) != 0 + || ((modifiers & (Modifier.PUBLIC | Modifier.PROTECTED)) == 0 +&& packageCanNotBeAddedAnotherClass(declaringClass)) + && !GroovyObject.class.isAssignableFrom(declaringClass)) { + securityManager.checkPermission(REFLECT_PERMISSION); +} +else if ((modifiers & (Modifier.PROTECTED)) != 0 + && declaringClass.equals(ClassLoader.class)){ + securityManager.checkCreateClassLoader(); + } + } + } + + private static boolean packageCanNotBeAddedAnotherClass(Class declaringClass) { + return declaringClass.getName().startsWith("java."); + } + + static public void checkAccessPermission(Method method) { --- End diff -- `public` isn't needed --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #533: GROOVY-7840: Verifier#makeDescriptorWithoutReturnT...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/533#discussion_r116363909 --- Diff: src/main/org/apache/groovy/ast/tools/MethodNodeUtils.java --- @@ -0,0 +1,66 @@ +/* + * 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 org.apache.groovy.ast.tools; + +import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.Parameter; + +public class MethodNodeUtils { +/** + * Return the method node's descriptor including its + * name and parameter types without generics. + * + * @param mNode the method node + * @return the method node's abbreviated descriptor excluding the return type + */ +public static String methodDescriptorWithoutReturnType(MethodNode mNode) { --- End diff -- This seems like it could be an instance method such as `MethodNode#getSignature()` ([JLS 8.4.2](http://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html#jls-8.4.2)) or `MethodNode#getTypeDescriptorWithoutReturnType()`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #533: GROOVY-7840: Verifier#makeDescriptorWithoutReturnT...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/533#discussion_r116363829 --- Diff: src/main/org/apache/groovy/ast/tools/MethodNodeUtils.java --- @@ -0,0 +1,66 @@ +/* + * 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 org.apache.groovy.ast.tools; + +import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.Parameter; + +public class MethodNodeUtils { +/** + * Return the method node's descriptor including its + * name and parameter types without generics. + * + * @param mNode the method node + * @return the method node's abbreviated descriptor excluding the return type + */ +public static String methodDescriptorWithoutReturnType(MethodNode mNode) { +StringBuilder sb = new StringBuilder(); +mNode.getTypeDescriptor(); +sb.append(mNode.getName()).append(':'); +for (Parameter p : mNode.getParameters()) { + sb.append(ClassNodeUtils.formatTypeName(p.getType())).append(','); +} +return sb.toString(); +} + +/** + * Return the method node's descriptor which includes its return type, + * name and parameter types without generics. + * + * @param mNode the method node + * @return the method node's descriptor + */ +public static String methodDescriptor(MethodNode mNode) { --- End diff -- This could be the body of `MethodNode#getTypeDescriptor` instance method. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #533: GROOVY-7840: Verifier#makeDescriptorWithoutReturnT...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/533#discussion_r116363767 --- Diff: src/main/org/apache/groovy/ast/tools/ClassNodeUtils.java --- @@ -0,0 +1,48 @@ +/* + * 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 org.apache.groovy.ast.tools; + +import org.codehaus.groovy.ast.ClassNode; + +public class ClassNodeUtils { +/** + * Formats a type name into a human readable version. For arrays, appends "[]" to the formatted + * type name of the component. For unit class nodes, uses the class node name. + * + * @param cNode the type to format + * @return a human readable version of the type name (java.lang.String[] for example) + */ +public static String formatTypeName(ClassNode cNode) { --- End diff -- This seems like a candidate to be a package-private instance method on `ClassNode`. Package-private part assumes the moving of the `MethodNodeUtils` methods into `MethodNode`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #:
Github user jwagenleitner commented on the pull request: https://github.com/apache/groovy/commit/0fb89906aa587920d11fae063bba1d1f8fe26254#commitcomment-21969276 I created [GROOVY-8171](https://issues.apache.org/jira/browse/GROOVY-8171) because there is a slight difference in behavior between the parsers when escaping the opening dollar slashy. Not sure if it's really a problem or not. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #:
Github user jwagenleitner commented on the pull request: https://github.com/apache/groovy/commit/0fb89906aa587920d11fae063bba1d1f8fe26254#commitcomment-21963417 Thanks, I had forgot to test with antlr4 enabled. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #531: fix spec test
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/531 fix spec test You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy dollarslashy Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/531.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 #531 commit ffe47e95d766359833a4463fe3d8ad26dafcd17f Author: John Wagenleitner Date: 2017-04-30T20:14:24Z fix spec test --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #530: Clarify documentation around indy
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/530 Clarify documentation around indy From a recent [user mailing list question](http://mail-archives.apache.org/mod_mbox/groovy-users/201704.mbox/%3c44315a6eb40a3769a12a5bfe465da...@posteo.de%3E) and related to PR groovy/groovy-website#124. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy indy-docs Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/530.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 #530 commit 055d1013e66b95dc18c2b367ffb3d797e67c9639 Author: John Wagenleitner Date: 2017-04-30T18:07:34Z Clarify documentation around indy --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #528: GROOVY-7579: Improve docs for invokeMethod
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/528 GROOVY-7579: Improve docs for invokeMethod You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy7579 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/528.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 #528 commit 79cd42d410310f6d2ea34a13e78d621dc2e658a9 Author: John Wagenleitner Date: 2017-04-23T23:06:13Z GROOVY-7579: Improve docs for invokeMethod --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #524: GROOVY-8156: Compile error when ListenerList annot...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/524 GROOVY-8156: Compile error when ListenerList annotation exists You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8156 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/524.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 #524 commit 1d24e37353ef1544a2d5a6fdb7929b1a46c598f3 Author: John Wagenleitner Date: 2017-04-14T20:33:59Z GROOVY-8156: Compile error when ListenerList annotation exists --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #523: cleanup now that jdk7 is baseline
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/523 cleanup now that jdk7 is baseline Avoids reflective checks/constructions where it is no longer needed since 7 is now baseline in 2_5_X and master. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy cleanup-jdk7 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/523.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 #523 commit 0982b7927f463cc98f8019576a7e1375e8d7a2c6 Author: John Wagenleitner Date: 2017-04-12T02:43:14Z cleanup now that jdk7 is baseline --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #522: GROOVY-8144: Invoking a public method declared in ...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/522 GROOVY-8144: Invoking a public method declared in a non-public class ⦠â¦result in a IllegalAccessError Commit 1a4c9918a4f12e64 introduced the DecompiledClassNode as part of enabling the ASM class resolver. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8144 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/522.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 #522 commit 3b6c2a504119e99d86de76019f48e0059b8fcf9e Author: John Wagenleitner Date: 2017-04-10T20:10:25Z GROOVY-8144: Invoking a public method declared in a non-public class result in a IllegalAccessError Commit 1a4c9918a4f12e64 introduced the DecompiledClassNode as part of enabling the ASM class resolver. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #520: GROOVY-8140: Invoke method not returning MOP super...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/520 GROOVY-8140: Invoke method not returning MOP super method if isCallTo⦠â¦Super See [dev mailing list discussion](http://mail-archives.apache.org/mod_mbox/groovy-dev/201703.mbox/%3CCAPbPdObHqPg7aFHSwLf0sRMdHqrRa1vuNwRvxuZVQu5VQ840%2Bw%40mail.gmail.com%3E) You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8140 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/520.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 #520 commit 433eaf8c62f809d531bb9984b126252172f8287c Author: John Wagenleitner Date: 2017-04-02T15:34:47Z GROOVY-8140: Invoke method not returning MOP super method if isCallToSuper --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #501: GROOVY-7248: MissingPropertyException: No such pro...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/501 GROOVY-7248: MissingPropertyException: No such property in finally block You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy7248 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/501.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 #501 commit 52e6a5b435651050d978260381496530b79eab1d Author: John Wagenleitner Date: 2017-02-20T15:52:13Z GROOVY-7248: MissingPropertyException: No such property in finally block --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #500: GROOVY-8056: GroovyCodeSource(URL) can leak a file...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/500 GROOVY-8056: GroovyCodeSource(URL) can leak a file handler URLConnect.getContentEncoding returns the Content-Encoding HTTP Header [1] which is not a charset. Since this method would have either returned null or an invalid charset, the code path specifying the encoding would normally not have been executed. The charset may be contained in the Content-Type header, but rather than attempt to parse that string which would require closing the connection, this fix avoids opening the connection and relies on the default charset. [1] https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.11 You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8056-content-encoding Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/500.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 #500 commit 29a641ccc212d397bdc11d3a995763b88dfe34b5 Author: John Wagenleitner Date: 2017-02-19T00:22:49Z GROOVY-8056: GroovyCodeSource(URL) can leak a file handler URLConnect.getContentEncoding returns the Content-Encoding HTTP Header [1] which is not a charset. Since this method would have either returned null or an invalid charset, the code path specifying the encoding would normally not have been executed. The charset may be contained in the Content-Type header, but rather than attempt to parse that string which would require closing the connection, this fix avoids opening the connection and relies on the default charset. [1] https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.11 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #496: Add a ASMifier tab to AstBrowser
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/496#discussion_r100702338 --- Diff: subprojects/groovy-console/src/main/groovy/groovy/inspect/swingui/AstBrowser.groovy --- @@ -403,9 +420,13 @@ class AstBrowser { def tabPane = mainSplitter.bottomComponent int tabCount = tabPane.getTabCount() for (int i = 0; i < tabCount; i++) { -if (bytecodeView.is(tabPane.getComponentAt(i))) { +def component = tabPane.getComponentAt(i); +if (bytecodeView.is(component)) { tabPane.setTitleAt(i, getByteCodeTitle()) break +} else if (asmifierView.is(component)) { +tabPane.setTitleAt(i, getASMifierTitle()) +break --- End diff -- The `ASMifier` tab title doesn't update because of the `breaks`, if both `breaks` are removed that should fix 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #489: GROOVY-8067: Possible deadlock when creating new C...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/489#discussion_r100162935 --- Diff: subprojects/stress/src/test/java/org/codehaus/groovy/reflection/ClassInfoDeadlockStressTest.java --- @@ -0,0 +1,140 @@ +/* + * 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 org.codehaus.groovy.reflection; + +import groovy.lang.GroovyClassLoader; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.groovy.stress.util.GCUtils; +import org.junit.Test; +import static org.junit.Assert.*; + +/** + * Tests for deadlocks in the ClassInfo caching. + * + */ +public class ClassInfoDeadlockStressTest { + +private static final int DEADLOCK_TRIES = 8; +private static final int THREAD_COUNT = 8; + +private final CountDownLatch startLatch = new CountDownLatch(1); +private final CountDownLatch completeLatch = new CountDownLatch(THREAD_COUNT); +private final GroovyClassLoader gcl = new GroovyClassLoader(); +private final AtomicInteger counter = new AtomicInteger(); + +/** + * We first generate a large number of ClassInfo instances for classes + * that are no longer reachable. Then queue up threads to all request + * ClassInfo instances for new classes simultaneously to ensure that + * clearing the old references wont deadlock the creation of new + * instances. + * + * GROOVY-8067 + */ +@Test +public void testDeadlock() throws Exception { +for (int i = 1; i <= DEADLOCK_TRIES; i++) { +System.out.println("Test Number: " + i); +generateGarbage(); +GCUtils.gc(); +attemptDeadlock(null); +} +} + +@Test +public void testRequestsForSameClassInfo() throws Exception { +Class newClass = createRandomClass(); +for (int i = 1; i <= DEADLOCK_TRIES; i++) { +System.out.println("Test Number: " + i); +generateGarbage(); +GCUtils.gc(); +attemptDeadlock(newClass); +} +ClassInfo newClassInfo = ClassInfo.getClassInfo(newClass); +for (ClassInfo ci : ClassInfo.getAllClassInfo()) { +if (ci.getTheClass() == newClass && ci != newClassInfo) { +fail("Found multiple ClassInfo instances for class"); +} +} +} + +private void attemptDeadlock(final Class cls) throws Exception { +for (int i = 0; i < THREAD_COUNT; i++) { +Runnable runnable = new Runnable() { +@Override +public void run() { +Class newClass = (cls == null) ? createRandomClass() : cls; +try { +startLatch.await(); +} catch (InterruptedException ie) { --- End diff -- can use `ThreadUtils.awaite()` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #489: GROOVY-8067: Possible deadlock when creating new C...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/489#discussion_r100162813 --- Diff: src/main/org/codehaus/groovy/util/ManagedConcurrentLinkedQueue.java --- @@ -0,0 +1,187 @@ +/* + * 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 org.codehaus.groovy.util; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; +import java.util.concurrent.ConcurrentLinkedQueue; + +/** + * A queue that stores the values wrapped in a Reference, the type of which is + * determined by the provided {@link ReferenceBundle}. Values stored in this queue + * that are put on the {@code ReferenceQueue} will be removed from the list when + * reference processing for the {@code ReferenceQueue} is done. + * + * This queue is backed by a {@link ConcurrentLinkedQueue} and is thread safe. The + * iterator will only return non-null values (reachable) and is based on the + * "weakly consistent" iterator of the underlying {@link ConcurrentLinkedQueue}. + * + * @param the type of values to store + */ +public class ManagedConcurrentLinkedQueue implements Iterable { + +private final ReferenceBundle bundle; +private final ConcurrentLinkedQueue> queue; + +/** + * Creates an empty ManagedConcurrentLinkedQueue that will use the given + * {@code ReferenceBundle} to store values as the given Reference + * type. + * + * @param bundle used to create the appropriate Reference type + * for the values stored + */ +public ManagedConcurrentLinkedQueue(ReferenceBundle bundle) { +this.bundle = bundle; +this.queue = new ConcurrentLinkedQueue>(); +} + +/** + * Adds the specified value to the queue. + * + * @param value the value to add + */ +public void add(T value) { +Element e = new Element(value); +queue.offer(e); +} + +/** + * Returns {@code true} if this queue contains no elements. + * + * This method does not check the elements to verify they contain + * non-null reference values. + */ +public boolean isEmpty() { +return queue.isEmpty(); +} + +/** + * Returns an array containing all values from this queue in the sequence they + * were added. + * + * @param tArray the array to populate if big enough, else a new array with + * the same runtime type + * @return an array containing all non-null values in this queue + */ +public T[] toArray(T[] tArray) { +return values().toArray(tArray); +} + +/** + * Returns an unmodifiable list containing all values from this queue in the + * sequence they were added. + */ +public List values() { +Iterator itr = iterator(); +if (!itr.hasNext()) { +return Collections.emptyList(); +} +List result = new ArrayList(100); +result.add(itr.next()); +while (itr.hasNext()) { +result.add(itr.next()); +} +return Collections.unmodifiableList(result); +} + +/** + * Returns an iterator over all non-null values in this queue. The values should be + * returned in the order they were added. + */ +@Override +public Iterator iterator() { +return new Iter(queue.iterator()); +} + +private final class Element extends ManagedReference { + +Element(V value) { +super(bundle, value); +} + +@Override +public void finalizeReference()
[GitHub] groovy pull request #491: GROOVY-8068: improper logging in groovy.sql.Sql
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/491#discussion_r99946115 --- Diff: subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java --- @@ -578,17 +578,26 @@ public static Sql newInstance(Map args) throws SQLException, Cla Object url = sqlArgs.remove("url"); Connection connection; +LOG.fine("url = " + url); if (props != null) { -System.err.println("url = " + url); -System.err.println("props = " + props); -connection = DriverManager.getConnection(url.toString(), new Properties(props)); +Properties propsCopy = new Properties(props); +connection = DriverManager.getConnection(url.toString(), propsCopy); +if (propsCopy.containsKey("password")) { +// don't log the password +propsCopy = new Properties(propsCopy); --- End diff -- That makes sense, I missed that the copy was passed to the DriverManager. Sorry for the noise. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #484: GROOVY-8067: Possible deadlock when creating new C...
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/484 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #491: GROOVY-8068: improper logging in groovy.sql.Sql
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/491#discussion_r99844410 --- Diff: subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java --- @@ -578,17 +578,26 @@ public static Sql newInstance(Map args) throws SQLException, Cla Object url = sqlArgs.remove("url"); Connection connection; +LOG.fine("url = " + url); if (props != null) { -System.err.println("url = " + url); -System.err.println("props = " + props); -connection = DriverManager.getConnection(url.toString(), new Properties(props)); +Properties propsCopy = new Properties(props); +connection = DriverManager.getConnection(url.toString(), propsCopy); +if (propsCopy.containsKey("password")) { +// don't log the password +propsCopy = new Properties(propsCopy); --- End diff -- I don't think this is needed since it's already been copied? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #490: GROOVY-8072: AstBrowser source view does not gener...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/490 GROOVY-8072: AstBrowser source view does not generate labels for stat⦠â¦ements You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8072 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/490.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 #490 commit 757db0c9e69c237a6416e6ddff1d81a20818fd23 Author: John Wagenleitner Date: 2017-02-05T17:37:06Z GROOVY-8072: AstBrowser source view does not generate labels for statements --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #489: GROOVY-8067: Possible deadlock when creating new C...
Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/489#discussion_r99482851 --- Diff: src/main/org/codehaus/groovy/reflection/ClassInfo.java --- @@ -186,30 +185,20 @@ public void setStrongMetaClass(MetaClass answer) { MetaClass strongRef = strongMetaClass; if (strongRef instanceof ExpandoMetaClass) { - ((ExpandoMetaClass)strongRef).inRegistry = false; - synchronized(modifiedExpandos){ -for (Iterator it = modifiedExpandos.iterator(); it.hasNext(); ) { - ClassInfo info = it.next(); - if(info == this){ -it.remove(); - } +((ExpandoMetaClass)strongRef).inRegistry = false; +for (Iterator itr = modifiedExpandos.iterator(); itr.hasNext(); ) { +ClassInfo info = itr.next(); +if(info == this) { +itr.remove(); +} } - } } strongMetaClass = answer; if (answer instanceof ExpandoMetaClass) { - ((ExpandoMetaClass)answer).inRegistry = true; - synchronized(modifiedExpandos){ -for (Iterator it = modifiedExpandos.iterator(); it.hasNext(); ) { - ClassInfo info = it.next(); -if(info == this){ - it.remove(); -} - } - modifiedExpandos.add(this); - } --- End diff -- Removed because it appeared to be a duplicate of the logic above. Only if the previous previous `strongMetaClass` value was an expando would it have been added to the map, so the iteration/removal above should have taken care to remove this `ClassInfo` from the `modifiedExpandos` and there's no reason to iterate again. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #489: GROOVY-8067: Possible deadlock when creating new C...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/489 GROOVY-8067: Possible deadlock when creating new ClassInfo entries in the cache As suggested in PR #484 removed the locking on the `ManagedLinkedList` by creating a new `ManagedConcurrentLinkedQueue`. Also added a `stress` subproject for tests that employ many threads, need GC, or just in general try to break things and take a long time. These require a special property to be set in order to run, otherwise they will be skipped. I tried to work it out in the `performance` subproject, but that seems to be very specialized for the compiler tests. Open to suggestions on a better way to handle these types of tests. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8067-mclq Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/489.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 #489 commit bb2464a919a3655f36707fa72fb30080c92a7288 Author: John Wagenleitner Date: 2017-02-05T06:13:26Z GROOVY-8067: Possible deadlock when creating new ClassInfo entries in the cache --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #484: GROOVY-8067: Possible deadlock when creating new C...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/484 GROOVY-8067: Possible deadlock when creating new ClassInfo entries in the cache While I have been able to replicate the deadlock between `GroovyClassValuePreJava7$Segment` and the `GlobalClassSet#add`, I have not directly observed one between the `modifiedExpandos` and the `GlobalClassSet`, but think that it would be good to isolate their reference processing too since both lock in their operations. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy8067 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/484.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 #484 commit 78f5aa0b5977919ba05dcad9fe8a7ee496abf2e8 Author: John Wagenleitner Date: 2017-01-29T18:26:43Z GROOVY-8067: test for demo only (DO NOT COMMIT) commit 58a27e7b1c437d436636658a5de9537fda5560d6 Author: John Wagenleitner Date: 2017-01-29T19:34:55Z GROOVY-8067: Possible deadlock when creating new ClassInfo entries in the cache --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #325: GROOVY-7646 - Classes generated by Eval() never co...
Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/325 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #477: AstBrowser fix showing Trait object initializers
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/477 AstBrowser fix showing Trait object initializers Fix to problem reported by @paulk-asert in PR #473 You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy4636-fix-traits Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/477.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 #477 commit afe6390d13731fbb93a7a4ea64157400ce9ea913 Author: John Wagenleitner Date: 2017-01-14T21:51:06Z AstBrowser fix for Trait object initializers Fix to GROOVY-4636 that incorrectly handled adding statements to the AST node tree. commit f8cd15277db43d9082b3d3af86c3d78730d4ccc6 Author: John Wagenleitner Date: 2017-01-14T22:09:59Z minor cleanup and refactoring --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #475: GROOVY-5471: Add "indy" option to Groovy Console (...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/475 GROOVY-5471: Add "indy" option to Groovy Console (and AstBrowser) You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy5471 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/475.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 #475 commit 8a8fd94a09491f24c692df5aa132de9c34bed70d Author: John Wagenleitner Date: 2017-01-03T06:42:21Z GROOVY-5471: Add "indy" option to Groovy Console (and AstBrowser) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #473: GROOVY-4636: AST Browser does not show object init...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/473 GROOVY-4636: AST Browser does not show object initializer statements You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy groovy4636 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/473.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 #473 commit 6be1eefa1790134c9d352dd6518fe281826617c0 Author: John Wagenleitner Date: 2016-12-31T18:47:32Z GROOVY-4636: AST Browser does not show object initializer statements --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #462: GROOVY-6175: invoking Closure property like method...
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/462 GROOVY-6175: invoking Closure property like method fails because of doCall Per the Closure class docs, to be able to use a Closure in short form, e.g., c(), in subclasses, you need to provide a doCall method with any signature you want to. This ensures that getMaximumNumberOfParameters() and getParameterTypes() will work too without any additional code. If no doCall method is provided a closure must be used in its long form, e.g., c.call(). You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy GROOVY-6175 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/462.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 #462 commit 76cffc9bab8825389258034cc0c231603ebe619e Author: John Wagenleitner Date: 2016-11-22T03:54:10Z GROOVY-6175: invoking Closure property like method fails because of doCall/call asynchronity Per the Closure class docs, to be able to use a Closure in short form, e.g., c(), in subclasses, you need to provide a doCall method with any signature you want to. This ensures that getMaximumNumberOfParameters() and getParameterTypes() will work too without any additional code. If no doCall method is provided a closure must be used in its long form, e.g., c.call(). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] groovy pull request #450: GROOVY-7089: Base64 URL Safe encoder
GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/450 GROOVY-7089: Base64 URL Safe encoder You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy GROOVY-7089-Base64Url Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/450.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 #450 commit 621db0bdf96587301779cb660ed9968c558c0616 Author: John Wagenleitner Date: 2016-10-25T01:20:51Z GROOVY-7089: Base64 URL Safe encoder --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---