Bug 341499 - [compiler][null] allocate extra bits in all methods of UnconditionalFlowInfo
Summary: [compiler][null] allocate extra bits in all methods of UnconditionalFlowInfo
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.7 M7   Edit
Assignee: Stephan Herrmann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-31 11:05 EDT by Stephan Herrmann CLA
Modified: 2011-04-25 06:23 EDT (History)
2 users (show)

See Also:


Attachments
proposed patch (3.37 KB, patch)
2011-04-05 16:31 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-03-31 11:05:11 EDT
The patch in bug 247564 contains a few fixes that are actually independent
of the issue of field references: currently some methods in 
UnconditionalFlowInfo fail to allocate the extra bits used for handling more
than 64 locals, namely:
  markAsDefinitelyNonNull
  markAsDefinitelyNull
  markAsDefinitelyUnknown

IMO these fixes should be released for 3.7.
Comment 1 Olivier Thomann CLA 2011-04-04 13:51:36 EDT
This makes sense.
Stephan, could you please prepare a patch for this ?
Comment 2 Stephan Herrmann CLA 2011-04-05 16:31:11 EDT
Created attachment 192598 [details]
proposed patch

This is the part of the patch from bug 247564 that will make the
markAsDefinitely* methods safe wrt array allocation.

While trying to create tests for this I found that in the current
implementation the new blocks seem to be unreachable because for locals
assignment will always(?) be marked before null status. This is similar to
our findings in bug 333089 comment 10. As argued there this is, however,
a fragile contract. Additionally, null analysis for either fields (bug 247564)
or parameters (using annotations - bug 186342) will trigger the new sections.

So, although I cannot demonstrate that things are currently broken, I'd still
like to release the patch now, OK?
Comment 3 Ayushman Jain CLA 2011-04-06 02:16:56 EDT
(In reply to comment #2)
> So, although I cannot demonstrate that things are currently broken, I'd still
> like to release the patch now, OK?

If by this you mean you cannot extract a test case to demo that something got fixed with the patch, I guess its ok because the testing for fields done for bug 247564 will also be a test for this fix. So you can go ahead and release this.
Just to double check, is there any additional change apart from the ones already in patch for bug 247564?
Comment 4 Stephan Herrmann CLA 2011-04-07 06:12:46 EDT
(In reply to comment #3)
> If by this you mean you cannot extract a test case to demo that something got
> fixed with the patch, I guess its ok because the testing for fields done for
> bug 247564 will also be a test for this fix.

Yes. Also one of the tests for null annotations now goes into that realm:
 http://dev.eclipse.org/viewcvs/viewvc.cgi/trunk/testplugins/org.eclipse.objectteams.jdt.nullity.tests/src/org/eclipse/objectteams/jdt/nullity/tests/NullAnnotationTest.java?root=TOOLS_OBJECTTEAMS&revision=1421&view=markup#l144
see test_nonnull_parameter_003(). This test throws NPE without the patch
and passes with it.

> Just to double check, is there any additional change apart from the ones
> already in patch for bug 247564?

No additional changes.
Comment 5 Stephan Herrmann CLA 2011-04-07 06:25:08 EDT
Released to HEAD for 3.7M7
Also released to BETA_JAVA7.
Comment 6 Ayushman Jain CLA 2011-04-25 06:23:30 EDT
Verified for 3.7M7 using code inspection