Bug 128806 - [compiler][null][correlation] synchronous null status (with ternary operator)
Summary: [compiler][null][correlation] synchronous null status (with ternary operator)
Status: CLOSED DUPLICATE of bug 538421
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.2   Edit
Hardware: All All
: P5 enhancement (vote)
Target Milestone: 4.7 M1   Edit
Assignee: Ayushman Jain CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2006-02-21 09:29 EST by Dominic Evans CLA
Modified: 2018-08-30 10:36 EDT (History)
3 users (show)

See Also:


Attachments
Tentative fix + tests (378.17 KB, patch)
2006-04-21 10:30 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 Dominic Evans CLA 2006-02-21 09:29:45 EST
Null reference analysis falsely states that on the final line of the following method (the return) "The variable editor may be null". This is not true as if editor was null then offset would be null and hence the method would have already returned before reaching that statement.

private AST getEditorAST(Editor editor) {
   Integer offset = editor == null ? null : editor.getCaretOffset();

   if (offset == null) {
      return null;
   }

   return editor.getAST(offset);
}

If you replace the ternary with a simple:

if (editor == null) {
	return null;
}

Then the warning goes away.

Rather oddly, if you have add this whilst also keeping the ternary then you get two warnings. One saying 'editor' in the ternary line cannot be null as it has already been checked against null, and one saying the 'editor' in the return line could be null.
Comment 1 Maxime Daniel CLA 2006-02-21 10:04:47 EST
This is a case in which you want one variable (offset) non null status to imply a non null status for another variable (editor).
We have made no attempt yet to tackle such function in a systematic way.
Comment 2 Maxime Daniel CLA 2006-03-21 12:15:31 EST
We do not plan any general variables correlation mechanism in 3.2 timeframe.
If this pattern is super frequent, we may consider a dedicated pattern, along similar lines as those described in bug 125319. Some constraints would be added though (aka editor would have to be final).
Is this something you use frequently?
Comment 3 Dominic Evans CLA 2006-03-22 04:32:57 EST
(In reply to comment #2)

Well this is generally the method we have adopted for condensing tests for null into as few statements as possible. It seems wrong to have for example:

...
child = parent.getChild();

if(child == null)
   return;

sibling = child.getNextSibling();

if(sibling == null)
   return;

goal = sibling.getNextSibling();

if(goal == null)
   return;
...

When you can do this with ternary as:

...
child = parent.getChild();
sibling = child == null ? null : child.getNextSibling();
goal = sibling == null ? null : sibling.getNextSibling();

if(goal == null)
   return;
...

Can you suggest a better Java programming approach?

Otherwise, it would seem a sensible case to add.
Comment 4 Maxime Daniel CLA 2006-03-22 08:18:59 EST
(In reply to comment #3)
I would consider like the following if you don't need the side effects on siblings and goal when then happen to be null:

...
if ((child = parent.getChild()) != null
    && (sibling = child.getNextSibling()) != null
    && (goal = sibling.getNextSibling()) != null) {
      // do something to goal
      return goal;
}
return null;

This is compact and efficient.
Comment 5 Maxime Daniel CLA 2006-04-21 10:30:52 EDT
Created attachment 39166 [details]
Tentative fix + tests

This patch:
- changes the states definitions for the null analysis (adds three);
- recodes transitions accordingly;
- augments functional tests for the null reference analysis;
- reorganizes (radically) implementation tests for the null reference analysis.
Comment 6 Maxime Daniel CLA 2006-04-21 10:32:50 EDT
Comment on attachment 39166 [details]
Tentative fix + tests

DO NOT USE: attached the patch to the wrong bug (127570).
Comment 7 Maxime Daniel CLA 2006-04-25 00:46:02 EDT
Post 3.2.
Comment 8 Maxime Daniel CLA 2007-04-04 09:45:40 EDT
*** Bug 180925 has been marked as a duplicate of this bug. ***
Comment 9 Maxime Daniel CLA 2007-06-19 08:06:06 EDT
Reopening as P5 (since RESOLVED LATER is deprecated).
Comment 10 Stephan Herrmann CLA 2016-06-28 17:17:53 EDT
Bulk closing all compiler bugs tagged [null][correlation], because we have no plans to add such a feature: it would be a tremendous implementation effort, beyond our current man power, and may be impossible to achieve within the desired performance bounds.

If s.o. has a viable strategy or even implementation for such a feature, I'm all ears.
Comment 11 Sasikanth Bharadwaj CLA 2016-08-02 04:46:22 EDT
Verified for 4.7 M1
Comment 12 Stephan Herrmann CLA 2018-08-30 10:36:38 EDT
I created a new umbrella RFE outlining what would be needed to address this issue.

*** This bug has been marked as a duplicate of bug 538421 ***