Summary: | [1.4][compiler] variable initialized within an assert expression are no longer reported as potential non initialized | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Olivier Thomann <Olivier_Thomann> | ||||||||
Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> | ||||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||||
Severity: | critical | ||||||||||
Priority: | P3 | CC: | srikanth_sankaran | ||||||||
Version: | 3.7 | Flags: | Olivier_Thomann:
review+
|
||||||||
Target Milestone: | 3.7 M3 | ||||||||||
Hardware: | PC | ||||||||||
OS: | Windows 7 | ||||||||||
Whiteboard: | |||||||||||
Attachments: |
|
Description
Olivier Thomann
2010-10-21 10:44:04 EDT
Created attachment 181440 [details]
proposed fix v1.0 + regression tests
This bug shows that using addInitializationsFrom(..) to add the initialization info from assert into the main stream wasn't the correct approach, since initializations taking place inside assert may/may not be honoured depending on whether asserts are enabled.
This patch uses mergedWith(..) instead, which instead of adding initializations blindly from the assert, combines those from the main stream and assert stream. This helps us in filtering variables which may not have been initialized in the main stream.
This patch passes all compiler tests. Olivier, requesting a quick review. Thanks! I think the test should check for this.complianceLevel >= ClassFileConstants.JDK1_4 instead of this.complianceLevel >= ClassFileConstants.JDK1_5 since assert are defined in 1.4. Beside this, it looks ok. Could you please move the tests to a public test suite ? I would recommend the tests to be moved to the class org.eclipse.jdt.core.tests.compiler.regression.AssertionTest. Then you don't actually need to check the compliance level. Created attachment 181442 [details]
Updated patch
Same patch with the tests moved to the AssertionTest suite.
Created attachment 181445 [details]
updated patch v2
Just noticed that there were two duplicate tests in the above patch - test018 and 020. So just removed one.
Released in HEAD for 3.7M3 Verified for 3.7 M3 using Build id: I20101025-1800 |