[jira] [Commented] (GROOVY-11263) Analyze dead code
[ https://issues.apache.org/jira/browse/GROOVY-11263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17801625#comment-17801625 ] ASF GitHub Bot commented on GROOVY-11263: - daniellansun commented on PR #2023: URL: https://github.com/apache/groovy/pull/2023#issuecomment-1873540757 @eric-milles As we all know, source code is meant for developers to read, and the less redundant code there is, the more developer-friendly it becomes. Admittedly, it's challenging to cover all scenarios of dead code, but at least we can support some common ones and gradually improve, which aligns with the principle of program evolution. As for ASM's analysis of dead code, at least I haven't seen any relevant API it offers. If you have any new findings, please share them with us. Thanks in advance. > Analyze dead code > - > > Key: GROOVY-11263 > URL: https://issues.apache.org/jira/browse/GROOVY-11263 > Project: Groovy > Issue Type: Improvement >Reporter: Daniel Sun >Priority: Major > Labels: breaking_change > Fix For: 5.x > > > Groovy allows dead code after {{throw}}, {{return}}, {{break}} and > {{continue}}, e.g. > {code:java} > def m() { >return >def a = 1 > } > {code} > It's better to avoid such dead code. -- This message was sent by Atlassian Jira (v8.20.10#820010)
Re: [PR] GROOVY-11263: Analyze dead code [groovy]
daniellansun commented on PR #2023: URL: https://github.com/apache/groovy/pull/2023#issuecomment-1873540757 @eric-milles As we all know, source code is meant for developers to read, and the less redundant code there is, the more developer-friendly it becomes. Admittedly, it's challenging to cover all scenarios of dead code, but at least we can support some common ones and gradually improve, which aligns with the principle of program evolution. As for ASM's analysis of dead code, at least I haven't seen any relevant API it offers. If you have any new findings, please share them with us. Thanks in advance. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@groovy.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GROOVY-11260: ASTMatcher should support matching a var-arg placeholder [groovy]
paulk-asert merged PR #2022: URL: https://github.com/apache/groovy/pull/2022 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@groovy.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (GROOVY-11260) ASTMatcher should support matching a var-arg placeholder
[ https://issues.apache.org/jira/browse/GROOVY-11260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17801622#comment-17801622 ] ASF GitHub Bot commented on GROOVY-11260: - paulk-asert merged PR #2022: URL: https://github.com/apache/groovy/pull/2022 > ASTMatcher should support matching a var-arg placeholder > > > Key: GROOVY-11260 > URL: https://issues.apache.org/jira/browse/GROOVY-11260 > Project: Groovy > Issue Type: Improvement >Reporter: Paul King >Assignee: Paul King >Priority: Major > Fix For: 5.x > > > Currently the ASTMatcher supports having a wildcard or placeholder term > within the AST tree pattern but it is cumbersome to do matching on a method > call that contains var-args. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (GROOVY-11263) Analyze dead code
[ https://issues.apache.org/jira/browse/GROOVY-11263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17801570#comment-17801570 ] ASF GitHub Bot commented on GROOVY-11263: - eric-milles commented on code in PR #2023: URL: https://github.com/apache/groovy/pull/2023#discussion_r1439064503 ## src/main/java/org/codehaus/groovy/classgen/Verifier.java: ## @@ -302,6 +310,12 @@ private void checkFinalVariables(final ClassNode node) { visitor.visitClass(node); } +private void checkDeadCode(final ClassNode node) { +if (null == sourceUnit) return; +GroovyClassVisitor visitor = new DeadCodeAnalyzer(sourceUnit); +visitor.visitClass(node); Review Comment: I think you can get to the `SourceUnit` through the `ClassNode` without having to change `Verifier` all around. > Analyze dead code > - > > Key: GROOVY-11263 > URL: https://issues.apache.org/jira/browse/GROOVY-11263 > Project: Groovy > Issue Type: Improvement >Reporter: Daniel Sun >Priority: Major > Labels: breaking_change > Fix For: 5.x > > > Groovy allows dead code after {{throw}}, {{return}}, {{break}} and > {{continue}}, e.g. > {code:java} > def m() { >return >def a = 1 > } > {code} > It's better to avoid such dead code. -- This message was sent by Atlassian Jira (v8.20.10#820010)
Re: [PR] GROOVY-11263: Analyze dead code [groovy]
eric-milles commented on code in PR #2023: URL: https://github.com/apache/groovy/pull/2023#discussion_r1439064503 ## src/main/java/org/codehaus/groovy/classgen/Verifier.java: ## @@ -302,6 +310,12 @@ private void checkFinalVariables(final ClassNode node) { visitor.visitClass(node); } +private void checkDeadCode(final ClassNode node) { +if (null == sourceUnit) return; +GroovyClassVisitor visitor = new DeadCodeAnalyzer(sourceUnit); +visitor.visitClass(node); Review Comment: I think you can get to the `SourceUnit` through the `ClassNode` without having to change `Verifier` all around. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@groovy.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GROOVY-11263: Analyze dead code [groovy]
eric-milles commented on PR #2023: URL: https://github.com/apache/groovy/pull/2023#issuecomment-1873409213 Besides the extra class bytecode, is there a strong reason for this? The issue ticket pops up out of the blue without much explanation. There are so many possible paths that I think this will be another feature that gets you a quick 80% of the way and then will have issue tickets forever with uncovered scenarios. Would it be easier to use a bytecode path analyzer? Something that has been through research and proving phases? I think ASM does some of this analysis already but does not provide any API to ask if a path is dead -- but it does know internally using path analysis. It replaces dead code with NOP instructions. Maybe just a NOP pruning is in order. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@groovy.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (GROOVY-11264) Bump jarjar to 1.13.1
[ https://issues.apache.org/jira/browse/GROOVY-11264?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17801532#comment-17801532 ] ASF GitHub Bot commented on GROOVY-11264: - daniellansun closed pull request #2024: GROOVY-11264: Bump jarjar to 1.13.1 URL: https://github.com/apache/groovy/pull/2024 > Bump jarjar to 1.13.1 > - > > Key: GROOVY-11264 > URL: https://issues.apache.org/jira/browse/GROOVY-11264 > Project: Groovy > Issue Type: Dependency upgrade >Reporter: Daniel Sun >Priority: Major > -- This message was sent by Atlassian Jira (v8.20.10#820010)
Re: [PR] GROOVY-11264: Bump jarjar to 1.13.1 [groovy]
daniellansun closed pull request #2024: GROOVY-11264: Bump jarjar to 1.13.1 URL: https://github.com/apache/groovy/pull/2024 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@groovy.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org