Bug 337738

Comment 1 Satyam Kandula 2011-02-22 00:22:07 EST

Created attachment 189456 [details]
Proposed patch

Comment 2 Satyam Kandula 2011-02-22 00:23:27 EST

Ayush, Can you please review the patch and give your comments.

Comment 3 Satyam Kandula 2011-02-22 07:01:37 EST

Ayush, I have released the fix on the BETA_JAVA7 branch, but please do review the changes so that I can incorporate the changes.

Comment 4 Ayushman Jain 2011-02-22 09:54:31 EST

The patch looks good. However, just to document some small concerns, I noticed a few things in checkCatchClause(), that are not part of the new consumeDisjunctive...() method.

1) The condition 
"if ((topKnownElementKind(COMPLETION_OR_ASSIST_PARSER) == K_BETWEEN_CATCH_AND_RIGHT_PAREN) && this.identifierPtr > -1) {
// NB: if the cursor is on the variable, then it has been reduced (so identifierPtr is -1),
//     thus this can only be a completion on the type of the catch clause"

infers completion on type of catch clause only when this.identifierPtr>-1. In the new scenario, identifierPtr is -1.


2) I'm not sure what the line this.lastCheckPoint = this.assistNode.sourceEnd + 1;
was for in the checkCatchClause() method. Do you have any idea?

Comment 5 Satyam Kandula 2011-02-23 08:47:29 EST

(In reply to comment #4)
> 1) The condition 
> "if ((topKnownElementKind(COMPLETION_OR_ASSIST_PARSER) ==
> K_BETWEEN_CATCH_AND_RIGHT_PAREN) && this.identifierPtr > -1) {
> // NB: if the cursor is on the variable, then it has been reduced (so
> identifierPtr is -1),
> //     thus this can only be a completion on the type of the catch clause"
> 
> infers completion on type of catch clause only when this.identifierPtr>-1. In
> the new scenario, identifierPtr is -1.
The getTypeReference() called from this code needs this identifier to create the appropriate reference. Now, the getTypeReference gets called from the parser directly which would have had the identifier on the stack at that moment. 
> 
> 2) I'm not sure what the line this.lastCheckPoint = this.assistNode.sourceEnd +
> 1;
> was for in the checkCatchClause() method. Do you have any idea?
Actually getTypeReference() is actually updating the lastCheckPoint and this doesn't look necessary.

Comment 6 Ayushman Jain 2011-06-29 06:44:10 EDT

Had forgotten to +1 this. So doing it now

Comment 7 Ayushman Jain 2011-06-29 06:57:47 EDT

Verified using Eclipse Java 7 Support(Beta) feature patch v20110623-0900.