Bug 176472 - [compiler][null] extraneous error in case of a labeled while(true) statement
Summary: [compiler][null] extraneous error in case of a labeled while(true) statement
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 3.3 M7   Edit
Assignee: Maxime Daniel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 178227 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-03-06 05:01 EST by Maxime Daniel CLA
Modified: 2007-04-27 10:11 EDT (History)
1 user (show)

See Also:

Attachments
Tentative fix + test cases (14.17 KB, patch)
2007-03-26 09:17 EDT, Maxime Daniel CLA 		no flags Details | Diff
Better fix + test cases (11.86 KB, patch)
2007-03-30 08:53 EDT, Maxime Daniel CLA 		no flags Details | Diff
Show Obsolete (1) 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 Maxime Daniel CLA 2007-03-06 05:01:45 EST 
I20070228-0930

Upon the following test case, JDT complains while it should not:

public class X {
  void foo(int i) {
	Object o = null;
	done: while (true) {
      switch (i) {
    	case 0:
    	  o = new Object();
    	  break;
    	case 1:
    	  break done;
      }
	}
	if (o == null) { // Redundant null check: The variable o can only be null at this location
	}
  }
  void foo(boolean b) {
	Object o = null;
	done: while (true) {
      if (b) {
        break done;
      }
	}
	if (o == null) { // Redundant null check: The variable o can only be null at this location
	}
  }
}

Released (inactive) test cases NullReferenceTest#460 and 461.
Comment 1 Maxime Daniel CLA 2007-03-06 09:20:00 EST 
The problem comes from the fact that we do not mix late null information into the break information of LabelFlowContext (and potentially others). The intervening contexts should bufferize the null implications of recordBreakFrom as they do for other null-related information. This is not a trivial change since it implies storing the flow information and the target for each break, and changing the logic used to climb up the chain of enclosing contexts.

Note that the second test case should be:
  void foo(boolean b) {
        Object o = null;
        done: while (true) {
          if (b) {
            break done;
          }
          o = new Object();
        }
        if (o == null) { // Redundant null check: The variable o can only be
null at this location
        }
  }

(To be more rigorous, we should even replace the test upon b by a test upon something that has a chance to change.)
Comment 2 Maxime Daniel CLA 2007-03-09 03:52:03 EST 
Not in 3.3 timeframe.
Comment 3 Maxime Daniel CLA 2007-03-15 04:20:07 EDT 
Reopening for moderate priority attempt in 3.3 timeframe.
Comment 4 Maxime Daniel CLA 2007-03-20 07:49:40 EDT 
*** Bug 178227 has been marked as a duplicate of this bug. ***
Comment 5 Maxime Daniel CLA 2007-03-26 09:17:05 EDT 
Created attachment 61971 [details]
Tentative fix + test cases
Comment 6 Maxime Daniel CLA 2007-03-26 09:17:53 EDT 
I believe the approach of the patch is sound. Kent, could you pls have a look?
Comment 7 Maxime Daniel CLA 2007-03-30 08:53:31 EDT 
Created attachment 62484 [details]
Better fix + test cases

Having a look with Philippe, we agreed that the approach was too complex (more over, it broke a test). This patch is much simpler and easier to understand. It also properly passes all existing tests, and remains on the safe side in that it only changes null related information - eliminating any risk to change the more general flow analysis.
Kent, Philippe, pls have a look and let me know what you think.
Comment 8 Kent Johnson CLA 2007-03-30 11:42:59 EDT 
fine with me
Comment 9 Maxime Daniel CLA 2007-04-02 07:06:38 EDT 
Released for 3.3 M7.
Comment 10 Olivier Thomann CLA 2007-04-27 10:11:57 EDT 
Verified for 3.3M7 using I20070427-0010