Bug 354554 - [null] conditional with redundant condition yields weak error message
Summary: [null] conditional with redundant condition yields weak error message
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: Other Linux
: P3 normal (vote)
Target Milestone: 3.8 M2   Edit
Assignee: Stephan Herrmann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-11 17:03 EDT by Stephan Herrmann CLA
Modified: 2011-09-13 11:50 EDT (History)
1 user (show)

See Also:


Attachments
Test & fix under test (3.50 KB, patch)
2011-08-11 17:19 EDT, Stephan Herrmann CLA
no flags Details | Diff
Same patch with copyrights updated (4.64 KB, patch)
2011-08-12 16:46 EDT, Stephan Herrmann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Herrmann CLA 2011-08-11 17:03:28 EDT
Given this snippet:

Object u = new Object();
Object r = (u == null ? u : null);
System.out.println(r.toString());

the compiler reports:
----------
1. ERROR in BugXYZ.java (at line 4) 
	Object r = (u == null ? u : null);
	            ^
Null comparison always yields false: The variable u cannot be null at this location
----------
2. ERROR in BugXYZ.java (at line 5)
	System.out.println(r.toString());
	                   ^
Potential null pointer access: The variable r may be null at this location
----------

The 2nd error is misleading since there's no way how r can be non-null.

The effect is caused by incomplete analysis of the conditional expression
after finding that the condition is always false.

When constructing the same using an if statement the dead branch would
simply be skipped by null analysis. That's why the issue cannot be
reproduced using if.
Comment 1 Stephan Herrmann CLA 2011-08-11 17:19:04 EDT
Created attachment 201357 [details]
Test & fix under test

This patch includes one more test, where the problem is even more
relevant: by swapping "==" for "!=" we get a warning where analysis
should indeed by silent.

The patch fixes the issue by ConditionalExpression.computeNullStatus
check the reachmodes of both branches, dropping the one that is not
reachable.

NullReferenceTests pass, more tests pending.
Comment 2 Stephan Herrmann CLA 2011-08-12 16:46:36 EDT
Created attachment 201437 [details]
Same patch with copyrights updated

All compiler and model tests passed with this patch.
Comment 3 Stephan Herrmann CLA 2011-08-12 16:49:02 EDT
Released in HEAD for 3.8M2.
Comment 4 Olivier Thomann CLA 2011-09-13 11:50:06 EDT
Verified for 3.8M2.