groovy
groovy copied to clipboard
apache
GROOVY-11263: Analyze dead code
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.
@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.
source code is meant for developers to read, and the less redundant code there is, the more developer-friendly it becomes.
So now you have a benefit statement that you can put in the ticket. It would be nice to have a period of review on the problem statement and cost-benefit analysis before being forced to review code in hand.
If a user does not care about the extra code to read and the extra bytecode generated, is there a way to turn this off?
Have you still left it as a compiler error or did you soften it to a warning? If analysis mis-identifies some code as dead code and fails compilation, what can a user do to get their previously-working code to compile?
What if a post-verify transform changes the code's structure? You have chosen a specific compile phase to do the analysis with no explanation as to why this is the step to use.
If you do not address such concerns, then why not leave this out as a global transform that you apply to your own code? Why must it be core code from the very start?
It's a -1 from me unless you can more carefully spell out the problem definition and the solution ramifications.
We can add an option to switch off dead code analysis. Also, dead code analysis, as its name shown, it just traverses AST and does not change the AST, so no tranforming changes involved.
Here is the table to show how some mainstream programming languages to handle dead code. Most of them prevent or warn the dead code, so I think it's good choice for Groovy to align with most of them.
| Language | Allows Consecutive return (Dead Code) |
Compiler/Interpreter Behavior |
|---|---|---|
| Java | No | Marks as unreachable code, may cause a compile error. |
| C++ | Yes | Might warn but allows compilation. |
| C# | No | Identifies unreachable code, causes a compile error. |
| Python | Yes | Does not prevent dead code, allows execution. |
| Scala | Yes | Might issue a warning but allows compilation. |
| Kotlin | No | Detects unreachable code, causes a compile error. |
| JavaScript | Yes | Does not check for dead code. |
dead code analysis, as its name shown, it just traverses AST and does not change the AST, so no tranforming [sic] changes involved.
This was understood. The point is that if you add a compiler error you may fail code that previously compiled. A compiler warning allows the user to be notified but continue using code unchanged. Also, you scan the AST at a specific point in time. If later AST transformations occur that address the dead code scenarios, you have false positive.
I'm just trying to have you describe your reasoning for when to do the analysis. Even the class generator does a bit of instruction re-ordering that may or may not introduce dead code paths. It seems the goal of this change is to identify any dead statements explicitly represented in the source file. If that is indeed the case, it would be good to state all of this in the original problem description.