Bug 338118

Summary:

[compiler] CastExpression type should be changed to be a type reference and not an expression

Product:

[Eclipse Project] JDT

Reporter:

Olivier Thomann <Olivier_Thomann>

Component:

Core

Assignee:

Olivier Thomann <Olivier_Thomann>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

deepakazad, srikanth_sankaran, stephan.herrmann

Version:

3.7

Flags:

stephan.herrmann: review+

Target Milestone:

3.7 M6

Hardware:

OS:

Windows 7

Whiteboard:

Attachments:

Description	Flags
Possible fix	none
Proposed fix	none

Description Olivier Thomann

2011-02-24 11:36:51 EST

By construction, the type is always a type reference. So we should make this clear inside the CastExpression type itself so that we can actually remove a bunch of instanceof checks and clean up some code.

Comment 1 Stephan Herrmann

2011-02-24 13:27:19 EST

That cleanup would be great, but in my understanding the parser 
can NOT guarantee that the type is actually a type 
(that's how I read consumeCastExpressionLL1()).

Are you thinking of making the grammar smarter or of converting the expression
into a type reference inside CastExpression(Expression,Expression) ?

Excuse my curiosity :)

Comment 2 Olivier Thomann

2011-02-24 13:45:05 EST

This is what I thought initially as well, but in order to get into consumeCastExpressionLL1(), it means that the rule InsideCastExpressionLL1 was called.
And this rule calls:
org.eclipse.jdt.internal.compiler.parser.Parser.consumeInsideCastExpressionLL1()
where the name is actually converted to a type reference.

So I believe that the parser can make sure that it is always a type reference. So there is no need to change the existing grammar.

Comment 3 Stephan Herrmann

2011-02-24 14:10:33 EST

(In reply to comment #2)

cool!

Comment 4 Olivier Thomann

2011-02-24 14:23:50 EST

Created attachment 189730 [details]
Possible fix

First draft.

Comment 5 Olivier Thomann

2011-02-24 14:30:08 EST

Another good reason to fix this is that this is actually mandatory to support jsr 308 (annotations on type).
Srikanth, I let you review the patch.
Thanks.

Comment 6 Olivier Thomann

2011-02-24 15:02:24 EST

Created attachment 189740 [details]
Proposed fix

Same as before with some additional cleanup.

Comment 7 Srikanth Sankaran

2011-02-25 00:24:19 EST

Olivier, is this the same issue discussed here:
http://dev.eclipse.org/mhonarc/lists/jdt-core-dev/msg01909.html
or a different one ?

Comment 8 Srikanth Sankaran

2011-02-25 00:26:18 EST

May be not, as that issue was resolved under https://bugs.eclipse.org/bugs/show_bug.cgi?id=292364.

I'll take a look.

Comment 9 Olivier Thomann

2011-02-25 08:48:07 EST

(In reply to comment #7)
> Olivier, is this the same issue discussed here:
> http://dev.eclipse.org/mhonarc/lists/jdt-core-dev/msg01909.html
> or a different one ?
yes, but to the difference that the fix for bug 292364 didn't go as far as this one does which means that the corresponding field inside the CastExpression class should be a type refererence and not a name reference.

Comment 10 Olivier Thomann

2011-02-28 13:28:42 EST

Released for 3.7M6.
Srikanth, this will be verified in the verification phase.

Comment 11 Stephan Herrmann

2011-03-08 09:48:45 EST

For the records:

For a minute I was puzzled about the additional casts to TypeReference
inside CompletionParser. It turns out these are safe based on the assumption
that consumeInsideCastExpression() has been called before, which in the
case of the CompletionParser indeed pushes a type reference onto the
expression stack.

Comment 12 Stephan Herrmann

2011-03-08 09:50:27 EST

Verified for 3.7 M6 via code inspection.