Bug 200158 - [compiler] inconsistent handling of unreachable code
Summary: [compiler] inconsistent handling of unreachable code
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: 3.3.2   Edit
Assignee: Maxime Daniel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-08-16 05:03 EDT by Mandel CLA
Modified: 2008-01-24 04:44 EST (History)
3 users (show)

See Also:
philippe_mulet: pmc_approved+
kent_johnson: review+


Attachments
Suggested fix (3.09 KB, patch)
2007-09-14 11:43 EDT, Maxime Daniel CLA
maxime_daniel: review?
Details | Diff
R3_3_MAINTENANCE patch (10.78 KB, patch)
2007-10-08 04:58 EDT, Maxime Daniel CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mandel CLA 2007-08-16 05:03:37 EDT
Build ID: I20070625-1500

Steps To Reproduce:
writing the following class, will detect an unreachable code part.

public class Test {

 private static final boolean PREDICATE = false;

  public Object test() throws Exception {
   if (PREDICATE) {
    Iterator i = new HashSet().iterator(); //is not detected as unreachable
    test:while (i.hasNext()) { //the test:-Label is essential to reproduce this bug
    }
    return null; //is detected as unreachable
   }
   return null;
  }
    
}

In fact this code is statically unreachable. Strictly regarded, the unreachable code part starts at the beginning of the conditional block. But this is not a reason to reject compilation.

If we omit the Label "test" from the while-statement, everything is fine.


More information:
Comment 1 Philipe Mulet CLA 2007-09-14 05:35:57 EDT
Reproduced. Indeed this is inconsistent. Thanks for reporting it.
Comment 2 Philipe Mulet CLA 2007-09-14 05:49:02 EDT
This regression got introduced during 3.3.
Comment 3 Philipe Mulet CLA 2007-09-14 05:50:25 EDT
Likely a consequence of fix for bug 134848
Comment 4 Philipe Mulet CLA 2007-09-14 06:03:34 EDT
Maxime - pls redo fix for bug 134848. It seems that the line "statementInfo.mergedWith(labelContext.initsOnBreak)" (in LabelStatement) should always be executed no matter what. Then the null info in some case could be managed separately afterwards. Currently the null info handling is breaking the standard flow analysis.

I would also want a fix for 3.3.2.
Comment 5 Philipe Mulet CLA 2007-09-14 07:07:11 EDT
There should be no difference between the test() and test1():

import java.util.*;

public class X {
	private static final boolean PREDICATE = false;
	public Object test() throws Exception {
		if (PREDICATE) {
			Iterator i = new HashSet().iterator(); // is not detected as unreachable
			test: while (i.hasNext()) { // the test:-Label is essential to reproduce this bug
			}
			return null; // is detected as unreachable
		}
		return null;
	}
	public Object test2() throws Exception {
		if (PREDICATE) {
			Iterator i = new HashSet().iterator(); // is not detected as unreachable
			while (i.hasNext()) {}
			return null; // is detected as unreachable
		}
		return null;
	}	
}
Comment 6 Maxime Daniel CLA 2007-09-14 08:58:58 EDT
Released FlowAnalysisTest#48 (inactive) and 49 in HEAD.
Comment 7 Maxime Daniel CLA 2007-09-14 11:43:14 EDT
Created attachment 78443 [details]
Suggested fix

Patch for HEAD. Tests pass. Kent could you please review it?
Comment 8 Maxime Daniel CLA 2007-09-20 04:27:34 EDT
Released for 3.4 M3.

Setting target to 3.3.2 per Philippe's comment #4.
Comment 9 Maxime Daniel CLA 2007-09-24 07:00:43 EDT
Reopening for 3.3.2.
Comment 10 Maxime Daniel CLA 2007-10-08 04:58:43 EDT
Created attachment 79879 [details]
R3_3_MAINTENANCE patch

Essentially porting the changes of the test infrastructure. The code change in core is the same as for HEAD.
Comment 11 Maxime Daniel CLA 2007-10-08 05:33:39 EDT
Released for 3.3.2.
Comment 12 Frederic Fusier CLA 2007-10-29 12:42:58 EDT
Verified for 3.4M3 using I20071029-0010 build.
Comment 13 Frederic Fusier CLA 2007-10-29 12:43:54 EDT
Sorry, I should not have set this bug as VERIFIED...
Comment 14 Frederic Fusier CLA 2007-10-29 12:44:44 EDT
...but set the status whiteboard instead!
Comment 15 Philipe Mulet CLA 2007-11-15 07:58:49 EST
This is a regression introduced when adding support for null analysis. The regression affects mandatory flow analysis diagnosis (not optional null analysis).

This is quite severe.
Comment 16 Philipe Mulet CLA 2007-11-23 09:16:50 EST
+1 for 3.3.2
Comment 17 Frederic Fusier CLA 2008-01-24 04:44:06 EST
Verified for 3.3.2 using build M20080123-0800.