Bug 256796 - [compiler] dead code detection: "if (DEBUG) return;" should also be trivial IF stmt
Summary: [compiler] dead code detection: "if (DEBUG) return;" should also be trivial I...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.8 M3   Edit
Assignee: Ayushman Jain CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 256463
Blocks:
  Show dependency tree
 
Reported: 2008-11-27 10:53 EST by Markus Keller CLA
Modified: 2011-10-25 02:44 EDT (History)
6 users (show)

See Also:

Attachments
proposed fix v1.0 + regression tests (11.12 KB, patch)
2011-09-29 14:28 EDT, Ayushman Jain CLA 		no flags Details | Diff
patch for NegativeTest (1.22 KB, patch)
2011-09-30 05:18 EDT, Ayushman Jain CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2008-11-27 10:53:01 EST 
+++ This bug was initially created as a clone of Bug #256463 +++

HEAD after bug 256463

COMPILER_PB_DEAD_CODE_IN_TRIVIAL_IF_STATEMENT should also suppress dead code problems *after* the if statement, not only in the else branch:

public class X {
    private static final boolean DEBUG= true;

    void foo() {
        if (DEBUG)
            return;
        else
            System.out.println("hello"); // not dead (good)
        System.out.println("world"); // dead (but should not be)
    }
    
    void bar() {
        if (DEBUG) {
            return;
        }
        System.out.println("hello"); // dead (but should not be)
    }
}
Comment 1 Philipe Mulet CLA 2008-11-28 04:48:58 EST 
This feels a corner situation to me.
Comment 2 Philipe Mulet CLA 2008-11-28 06:52:18 EST 
Thinking more of this, I believe this is actually an evil pattern, which shouldn't exist... so I wouldn't treat it gracefully.

        if (DEBUG)
            return;
Comment 3 Jerome Lanneluc CLA 2008-12-08 05:21:38 EST 
Reopen to assign
Comment 4 Jerome Lanneluc CLA 2008-12-08 05:22:36 EST 
(In reply to comment #2)
> Thinking more of this, I believe this is actually an evil pattern, which
> shouldn't exist... so I wouldn't treat it gracefully.
Comment 5 Jerome Lanneluc CLA 2008-12-09 07:05:25 EST 
Verified for 3.5M4
Comment 6 Dani Megert CLA 2011-03-15 10:17:22 EDT 
I disagree with the resolution. The field can have an arbitrary name/meaning and only adding support for the case where we the initial value is 'false' seems wrong to me.

Currently this bug is the main reason for the dead code warnings in JDT UI.
Comment 7 Ayushman Jain CLA 2011-09-29 14:28:53 EDT 
Created attachment 204316 [details]
proposed fix v1.0 + regression tests

This patch makes sure that the merged info from the if and else branches is not unreachable because of a known dead code pattern used as condition, when the COMPILER_PB_DEAD_CODE_IN_TRIVIAL_IF_STATEMENT is disabled. 

If the else statement is empty or does not otherwise return, the reachable bit of the flow info prior to if-else is taken as the bit for the subsequent info, otherwise the reachability is taken from the else statement
Comment 8 Ayushman Jain CLA 2011-09-30 05:18:09 EDT 
Created attachment 204355 [details]
patch for NegativeTest

Second part of patch from CVS
Comment 9 Ayushman Jain CLA 2011-09-30 05:22:10 EDT 
Fixed in HEAD with commit 5333a8d6e234b4d5bbcbee365cfa39aafade1032. I don't see the dead code warnings in jdt.ui anymore.
Comment 10 Dani Megert CLA 2011-10-03 08:58:44 EDT 
Thanks!
Comment 11 Srikanth Sankaran CLA 2011-10-25 02:44:47 EDT 
Verified for 3.8 M3 using build id: N20111022-2000