Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 348883 - [1.7] Fix breakages caused by the new UnionType node
Summary: [1.7] Fix breakages caused by the new UnionType node
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.7.1   Edit
Assignee: Deepak Azad CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-09 08:47 EDT by Deepak Azad CLA
Modified: 2011-08-02 05:45 EDT (History)
3 users (show)

See Also:

Attachments
fix + tests - I (45.33 KB, patch)
2011-06-09 08:47 EDT, Deepak Azad CLA 		no flags Details | Diff
tests - II (12.08 KB, patch)
2011-06-09 14:22 EDT, Deepak Azad CLA 		no flags Details | Diff
additional fix (5.90 KB, patch)
2011-06-20 06:51 EDT, Deepak Azad CLA 		no flags Details | Diff
fix + tests - III (11.42 KB, patch)
2011-06-27 02:35 EDT, Deepak Azad CLA 		no flags Details | Diff
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 Deepak Azad CLA 2011-06-09 08:47:01 EDT 
Created attachment 197692 [details]
fix + tests - I
Comment 1 Deepak Azad CLA 2011-06-09 14:22:56 EDT 
Created attachment 197718 [details]
tests - II

Tests for changes in MethodExitsFinder and ExceptionOccurrencesFinder which I had done last week.
Comment 2 Deepak Azad CLA 2011-06-20 06:51:19 EDT 
Created attachment 198244 [details]
additional fix

Looking at the code I think this change should be done, however I have not been able to narrow down a test case for this change.
Comment 3 Deepak Azad CLA 2011-06-20 09:09:15 EDT 
(In reply to comment #2)
> Created attachment 198244 [details] [diff]
> additional fix
> 
> Looking at the code I think this change should be done, however I have not been
> able to narrow down a test case for this change.

All our tests pass with this patch. While the code is executed, I do not think that its result is used in any meaningful way.
Comment 4 Markus Keller CLA 2011-06-24 12:07:38 EDT 
(In reply to comment #3)
Nasty riddle! Even if you change the two methods to "return true;" and "return;", all tests still pass.

Both methods only deal with FlowInfo#fExceptions. If you open a call hierarchy on that, the only real client is FlowInfo#hasUncaughtException(). I also agree that the while loops look wrong (should walk superclasses of exceptionType, not of catchedType).

However, I also could not come up with a counterexample. For 3.7.1, we better don't touch code that was in for a long time and that we don't fully understand.

Can you please open a new bug for 3.8 to remove FlowInfo#fExceptions and everything that refers to it? AFAICS, the ExceptionAnalyzer now handles that.
Comment 5 Deepak Azad CLA 2011-06-24 12:35:14 EDT 
(In reply to comment #4)
> Can you please open a new bug for 3.8 to remove FlowInfo#fExceptions and
> everything that refers to it? AFAICS, the ExceptionAnalyzer now handles that.
Thanks Markus! I filed Bug 350290.
Comment 6 Deepak Azad CLA 2011-06-27 02:35:30 EDT 
Created attachment 198621 [details]
fix + tests - III

Disable Generalize Type Refactoring for union types
- It is a bit of work to make the refactoring work correctly for Union types
- I don't think that the refactoring is important enough to spend much time on it now
- We can always fix it in future :)
Comment 7 Deepak Azad CLA 2011-06-27 02:45:38 EDT 
I am done here, hence marking this bug as fixed. Any issues found in future can be handled in separate bugs.
Comment 8 Michael Rennie CLA 2011-07-20 12:45:45 EDT 
Verified.