Bug 40421 - Unnecessary cast warning...true but...
Summary: Unnecessary cast warning...true but...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows 2000
: P3 normal (vote)
Target Milestone: 3.0 M3   Edit
Assignee: Philipe Mulet CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-07-17 15:17 EDT by Darin Swanson CLA
Modified: 2003-08-28 04:47 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Swanson CLA 2003-07-17 15:17:28 EDT
In org.eclipse.ant.core.AntRunner we have two methods run(Object) and 
run(IProgressMonitor).

Another method, calls run((IProgress)null).
From the "rules" this is an unnecessary cast but it is a bit of a head 
scratcher initially as it is not obvious which method will be called without 
the cast.  Not sure if there is anything that can be done here or not?
Comment 1 Philipe Mulet CLA 2003-07-17 15:20:32 EDT
In this case, there is no ambiguity, since one signature is more specific than 
the other, but I agree this isn't obvious. 

One thing you could do instead is write run(/*IProgress*/null).
Or I was thinking of tolerating unnecessary cast on 'null'...
Comment 2 Philipe Mulet CLA 2003-07-17 18:03:51 EDT
Actually, should only tolerate null cast for arguments.
Comment 3 Sebastian Davids CLA 2003-07-18 08:23:28 EDT
I prefer it to be flagged as an unneccessary cast.

If the user *really* needs this construct he could either use your

run(/*IProgress*/ null)

or write something like

IProgress progress = null;
run(progress)

@@@@

Actually I'm partial to using Null-Objects instead of "nulls" =D
Comment 4 Philipe Mulet CLA 2003-07-18 08:24:53 EDT
Actually, I like your proposed solution using extra variable.
Comment 5 Richard Kulp CLA 2003-07-18 11:59:38 EDT
I disagree. Using run((IProgressMonitor) null) is a very common and necessary
cast to make sure the correct method is called. There can be very different
actions if the wrong one is called. Doing run(/* IProgressMonitor */ null) is
ineffective. The wrong one, run(Object), will be called in this case because the
comment does nothing.

(Or are the java syntax rules such that run(null) will call
run(IProgressMonitor) instead of run(Object)? If that is the case, then what
happens if there is a run(Object), run(IProgessMonitor), and run(SomethingElse).
Which would get called? My thought would of been that null fits Object better
than the other two. The cast would be a very obvious and useful addition because
the rules would be confusing in this case without the casts.)

I also feel using an extra variable just to do this is a waste of time and
space. The cast is a very concise and obvious way of doing it.

The only time the cast is truly unnecessary is if there is only one signature
for the method, i.e. only run(Object).

This is for arguments casting. In assignments or expressions I agree it is
unnecessary because the target can be decided upon, while for overloaded methods
it can't be.

I'm responding as requested from the newsgroup. Thanks.
Comment 6 Philipe Mulet CLA 2003-07-18 12:41:56 EDT
Clarification: as per spec, the most specific signature will be chosen. Thus, 
in presence of 2 methods:

run(Object), run(IProgressMonitor)  --> run(null)-->run(IProgressMonitor)

In case a 3rd run(SomethingElse) is added, then it becomes ambiguous with run
(IProgressMonitor).

All the compiler is checking is that without the cast you would bind to the 
same method anyway. With nulls it is truly tricky...

Could add an option to signal/ignore casted nulls ?
Comment 7 Philipe Mulet CLA 2003-07-21 05:29:57 EDT
Tolerate null argument cast since they contain valuable information improving 
code readability.

Comment 8 Philipe Mulet CLA 2003-07-21 05:30:22 EDT
Fixed
Comment 9 David Audel CLA 2003-08-28 04:47:58 EDT
Verified.