Bug 279715 - [inline] Inline Constant and Inline Local Variable are missing parentheses for extended '-' chains
Summary: [inline] Inline Constant and Inline Local Variable are missing parentheses fo...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5.1   Edit
Assignee: Markus Keller CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-09 18:36 EDT by Markus Keller CLA
Modified: 2009-08-04 14:08 EDT (History)
2 users (show)

See Also:
daniel_megert: review+

Attachments
Test (2.57 KB, patch)
2009-06-16 13:09 EDT, Markus Keller CLA 		no flags Details | Diff
Fix (1.08 KB, patch)
2009-06-17 06:35 EDT, Markus Keller 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 Markus Keller CLA 2009-06-09 18:36:18 EDT 
I20090605-1444

Inline Constant and Inline Local Variable are missing parentheses for extended '-' chains:

public class Try {
    public static final int B= 12;
    public static final int C= B - 1; //inline C
    public static final int K= 99;
    
    public static void main(String[] args) {
//        int B= 12;
//        int C= B - 1; //inline C
//        int K= 99;
        
        int f1= K - 1 - C;
        int f2= K - C - C - C;
        
        int x1= K + C;
        int x2= K - C;
        int x3= K + 1 - C;
        int x4= K - 1 + C;
        int x5= K + 1 + C - C - C;
    }
}
Comment 1 Markus Keller CLA 2009-06-09 18:49:52 EDT 
The bug is in ASTNodes.substituteMustBeParenthesized(Expression, Expression), where this shortcut is invalid:

if (locationInParent instanceof ChildListPropertyDescriptor) {
    // e.g. argument lists of MethodInvocation, ClassInstanceCreation, ...
    return false;
} else

Fix is to replace the test by:

if (locationInParent instanceof ChildListPropertyDescriptor
        && locationInParent != InfixExpression.EXTENDED_OPERANDS_PROPERTY) {
Comment 2 Markus Keller CLA 2009-06-16 13:09:26 EDT 
Created attachment 139321 [details]
Test
Comment 3 Markus Keller CLA 2009-06-16 13:13:17 EDT 
Released to HEAD.

That's a nasty one, since the subtle error can be hard to spot (especially the off-by-1 case where I ran into this bug).

Dani, do you agree for 3.5.1?
Comment 4 Markus Keller CLA 2009-06-17 06:35:24 EDT 
Created attachment 139411 [details]
Fix
Comment 5 Dani Megert CLA 2009-06-17 06:56:31 EDT 
Patch is good and makes sense to fix in 3.5.1.
Comment 6 Dani Megert CLA 2009-06-25 08:55:22 EDT 
Fixed in R3_5_maintenance branch.
Comment 7 Raksha Vasisht CLA 2009-08-04 08:37:13 EDT 
Verified for 3.6 M1 using I20090803-1800

Filed bug 285565 to track a special case with tab size = 0.
Comment 8 Markus Keller CLA 2009-08-04 14:08:29 EDT 
Verified in M20090729-0903.