Bug 223685 - [clean-up] Necessary cast removed
Summary: [clean-up] Necessary cast removed
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 3.4 M7   Edit
Assignee: Philipe Mulet CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-24 13:01 EDT by S.G. CLA
Modified: 2008-04-28 10:57 EDT (History)
3 users (show)

See Also:


Attachments
Proposed patch (52.47 KB, patch)
2008-04-10 07:00 EDT, Philipe Mulet CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description S.G. CLA 2008-03-24 13:01:30 EDT
Build ID: M20071023-1652

Steps To Reproduce:
The following code is altered by the clean-up function to something that won't compile.

The original:

Integer a = 0;
char b = (char)((int)a);

Cleaned up:

Integer a = 0;
char b = (char)(a);

But you can't convert an "Integer" object to a char directly. It's probably "best practice" to use the intValue() function of the Integer object, but using clean-up should not result in code that won't compile.
Comment 1 Benno Baumgartner CLA 2008-03-25 14:34:23 EDT
The clean up removes all casts which are flagged as unnecessary by the compiler. I guess the compiler is wrong here as the cast is not unnecessary but the compiler flags it as unnecessary.

Moving to core
Comment 2 Olivier Thomann CLA 2008-03-25 14:39:10 EDT
I guess your project is using 1.5 settings.
Comment 3 S.G. CLA 2008-03-25 19:30:25 EDT
(In reply to comment #2)
> I guess your project is using 1.5 settings.
> 

If you are referring to the JRE/JDK I'm using it's sun-jdk-1.6.0.04
If you mean the compiler compliance level, it's set to 6.0
If you mean anything else, maybe, I have no idea
Comment 4 Philipe Mulet CLA 2008-04-09 07:53:37 EDT
Indeed the cast is necessary.

I wonder if a more general solution would also to stop treating cast as unnecessary as soon as they induce a boxing/unboxing conversion; e.g.

Integer a = 0;
int i = (int) a; // currently flagged as unnecessary

For documentation purpose, we should allow user to document their boxing/unboxing operation explicitly, like we do for explicit casts of null values (which is good practice in general).
Comment 5 Philipe Mulet CLA 2008-04-09 08:02:13 EDT
And maybe even go further as to stop signaling implicit boxing/unboxing when an explicit cast is speficied (i.e. it is no secret any longer).
Comment 6 Philipe Mulet CLA 2008-04-10 07:00:10 EDT
Thinking more about it, I would rather address this very scenario with 2 casts nested in each other.

Comment 7 Philipe Mulet CLA 2008-04-10 07:00:38 EDT
Created attachment 95498 [details]
Proposed patch
Comment 8 Philipe Mulet CLA 2008-04-10 07:02:57 EDT
Added AutoboxingTest#test153.

Patch is unfortunately made bigger due to simultaneous code cleanup/reformatting.
Real change was actually quite small.
Comment 9 Philipe Mulet CLA 2008-04-10 07:03:42 EDT
Released for 3.4M7.
Fixed
Comment 10 Jerome Lanneluc CLA 2008-04-28 10:57:51 EDT
Verified for 3.4M7 using I20080427-2000