Attachment 126152 Details for Bug 265142 – Patch with review comments incorporated

[patch] Patch with review comments incorporated

patch.txt (text/plain), 16.89 KB, created by Srikanth Sankaran

on 2009-02-19 06:09:13 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Srikanth Sankaran

Created: 2009-02-19 06:09:13 EST

Size: 16.89 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java,v
>retrieving revision 1.143
>diff -u -r1.143 MessageSend.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java	5 Feb 2009 09:38:11 -0000	1.143
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java	19 Feb 2009 11:06:23 -0000
>@@ -378,7 +378,7 @@
> 						}
> 						this.binding = closestMatch;
> 						MethodBinding closestMatchOriginal = closestMatch.original();
>-						if ((closestMatchOriginal.isPrivate() || closestMatchOriginal.declaringClass.isLocalType()) && !scope.isDefinedInMethod(closestMatchOriginal)) {
>+						if (closestMatchOriginal.isOrEnclosedByPrivateType() && !scope.isDefinedInMethod(closestMatchOriginal)) {
> 							// ignore cases where method is used from within inside itself (e.g. direct recursions)
> 							closestMatchOriginal.modifiers |= ExtraCompilerModifiers.AccLocallyUsed;
> 						}
>@@ -426,7 +426,7 @@
> 		if (closestMatch != null) {
> 			this.binding = closestMatch;
> 			MethodBinding closestMatchOriginal = closestMatch.original();
>-			if ((closestMatchOriginal.isPrivate() || closestMatchOriginal.declaringClass.isLocalType()) && !scope.isDefinedInMethod(closestMatchOriginal)) {
>+			if (closestMatchOriginal.isOrEnclosedByPrivateType() && !scope.isDefinedInMethod(closestMatchOriginal)) {
> 				// ignore cases where method is used from within inside itself (e.g. direct recursions)
> 				closestMatchOriginal.modifiers |= ExtraCompilerModifiers.AccLocallyUsed;
> 			}
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java,v
>retrieving revision 1.95
>diff -u -r1.95 QualifiedAllocationExpression.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java	5 Feb 2009 09:38:11 -0000	1.95
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java	19 Feb 2009 11:06:24 -0000
>@@ -343,7 +343,7 @@
> 							}
> 							this.binding = closestMatch;
> 							MethodBinding closestMatchOriginal = closestMatch.original();
>-							if ((closestMatchOriginal.isPrivate() || closestMatchOriginal.declaringClass.isLocalType()) && !scope.isDefinedInMethod(closestMatchOriginal)) {
>+							if (closestMatchOriginal.isOrEnclosedByPrivateType() && !scope.isDefinedInMethod(closestMatchOriginal)) {
> 								// ignore cases where method is used from within inside itself (e.g. direct recursions)
> 								closestMatchOriginal.modifiers |= ExtraCompilerModifiers.AccLocallyUsed;
> 							}
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java,v
>retrieving revision 1.79
>diff -u -r1.79 AllocationExpression.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java	5 Feb 2009 09:38:11 -0000	1.79
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java	19 Feb 2009 11:06:20 -0000
>@@ -321,7 +321,7 @@
> 						}
> 						this.binding = closestMatch;
> 						MethodBinding closestMatchOriginal = closestMatch.original();
>-						if ((closestMatchOriginal.isPrivate() || closestMatchOriginal.declaringClass.isLocalType()) && !scope.isDefinedInMethod(closestMatchOriginal)) {
>+						if (closestMatchOriginal.isOrEnclosedByPrivateType() && !scope.isDefinedInMethod(closestMatchOriginal)) {
> 							// ignore cases where method is used from within inside itself (e.g. direct recursions)
> 							closestMatchOriginal.modifiers |= ExtraCompilerModifiers.AccLocallyUsed;
> 						}
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java,v
>retrieving revision 1.102
>diff -u -r1.102 ConstructorDeclaration.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java	17 Feb 2009 12:35:54 -0000	1.102
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java	19 Feb 2009 11:06:21 -0000
>@@ -59,8 +59,25 @@
> 		if (constructorBinding.isPrivate()) {
> 			if ((this.binding.declaringClass.tagBits & TagBits.HasNonPrivateConstructor) == 0)
> 				break checkUnused; // tolerate as known pattern to block instantiation
>-		} else if ((this.binding.declaringClass.tagBits & (TagBits.IsAnonymousType|TagBits.IsLocalType)) != TagBits.IsLocalType) {
>+		} else if (!constructorBinding.isOrEnclosedByPrivateType()) {
> 			break checkUnused;
>+ 		}
>+		// https://bugs.eclipse.org/bugs/show_bug.cgi?id=264991, Don't complain about this
>+		// constructor being unused if the base class doesn't have a no-arg constructor.
>+		// See that a seemingly unused constructor that chains to another constructor with a
>+		// this(...) can be flagged as being unused without hesitation.
>+		// https://bugs.eclipse.org/bugs/show_bug.cgi?id=265142
>+		if (this.constructorCall.accessMode != ExplicitConstructorCall.This) {
>+			ReferenceBinding superClass = constructorBinding.declaringClass.superclass();
>+			if (superClass == null)
>+				break checkUnused;
>+			// see if there is a no-arg super constructor
>+			MethodBinding methodBinding = superClass.getExactConstructor(Binding.NO_PARAMETERS);
>+			if (methodBinding == null || !methodBinding.isValidBinding())
>+				break checkUnused;
>+			if (!methodBinding.canBeSeenBy(SuperReference.implicitSuperConstructorCall(), this.scope))
>+				break checkUnused;
>+			// otherwise default super constructor exists, so go ahead and complain unused.
> 		}
> 		// complain unused
> 		this.scope.problemReporter().unusedPrivateConstructor(this);
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/ExplicitConstructorCall.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ExplicitConstructorCall.java,v
>retrieving revision 1.70
>diff -u -r1.70 ExplicitConstructorCall.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/ExplicitConstructorCall.java	5 Feb 2009 09:38:11 -0000	1.70
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/ExplicitConstructorCall.java	19 Feb 2009 11:06:22 -0000
>@@ -376,7 +376,7 @@
> 							}
> 							this.binding = closestMatch;
> 							MethodBinding closestMatchOriginal = closestMatch.original();
>-							if ((closestMatchOriginal.isPrivate() || closestMatchOriginal.declaringClass.isLocalType()) && !scope.isDefinedInMethod(closestMatchOriginal)) {
>+							if (closestMatchOriginal.isOrEnclosedByPrivateType() && !scope.isDefinedInMethod(closestMatchOriginal)) {
> 								// ignore cases where method is used from within inside itself (e.g. direct recursions)
> 								closestMatchOriginal.modifiers |= ExtraCompilerModifiers.AccLocallyUsed;
> 							}
>@@ -403,7 +403,7 @@
> 				if (checkInvocationArguments(scope, null, receiverType, this.binding, this.arguments, argumentTypes, argsContainCast, this)) {
> 					this.bits |= ASTNode.Unchecked;
> 				}
>-				if (this.binding.isPrivate() || receiverType.isLocalType()) {
>+				if (this.binding.isOrEnclosedByPrivateType()) {
> 					this.binding.original().modifiers |= ExtraCompilerModifiers.AccLocallyUsed;
> 				}
> 				if (this.typeArguments != null
>#P org.eclipse.jdt.core.tests.compiler
>Index: src/org/eclipse/jdt/core/tests/compiler/regression/InnerEmulationTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/InnerEmulationTest.java,v
>retrieving revision 1.51
>diff -u -r1.51 InnerEmulationTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/InnerEmulationTest.java	17 Feb 2009 12:35:51 -0000	1.51
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/InnerEmulationTest.java	19 Feb 2009 11:06:39 -0000
>@@ -2996,18 +2996,13 @@
> 			"	      ^\n" +
> 			"The type B is never used locally\n" +
> 			"----------\n" +
>-			"2. WARNING in X.java (at line 8)\n" +
>-			"	B() {	\n" +
>-			"	^^^\n" +
>-			"The constructor B() is never used locally\n" +
>-			"----------\n" +
>-			"3. ERROR in X.java (at line 9)\n" +
>+			"2. ERROR in X.java (at line 9)\n" +
> 			"	super(new A(){	\n" +
> 			"				});	\n" +
> 			"	      ^^^^^^^^^^^^^^^\n" +
> 			"No enclosing instance of type X is available due to some intermediate constructor invocation\n" +
> 			"----------\n" +
>-			"4. WARNING in X.java (at line 9)\n" +
>+			"3. WARNING in X.java (at line 9)\n" +
> 			"	super(new A(){	\n" +
> 			"	          ^^^\n" +
> 			"Access to enclosing constructor A() is emulated by a synthetic accessor method\n" +
>@@ -3039,12 +3034,7 @@
> 			"	      ^\n" +
> 			"The type B is never used locally\n" +
> 			"----------\n" +
>-			"2. WARNING in X.java (at line 8)\n" +
>-			"	B() {	\n" +
>-			"	^^^\n" +
>-			"The constructor B() is never used locally\n" +
>-			"----------\n" +
>-			"3. ERROR in X.java (at line 9)\n" +
>+			"2. ERROR in X.java (at line 9)\n" +
> 			"	super(new A(){	\n" +
> 			"				});	\n" +
> 			"	      ^^^^^^^^^^^^^^^\n" +
>@@ -3136,29 +3126,24 @@
> 			"	      ^\n" +
> 			"The type B is never used locally\n" +
> 			"----------\n" +
>-			"2. WARNING in X.java (at line 8)\n" +
>-			"	B() {	\n" +
>-			"	^^^\n" +
>-			"The constructor B() is never used locally\n" +
>-			"----------\n" +
>-			"3. ERROR in X.java (at line 9)\n" +
>+			"2. ERROR in X.java (at line 9)\n" +
> 			"	super(new A(){	\n" +
> 			"					void foo() { System.out.println(X.this);	} \n" +
> 			"				});	\n" +
> 			"	      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
> 			"No enclosing instance of type X is available due to some intermediate constructor invocation\n" +
> 			"----------\n" +
>-			"4. WARNING in X.java (at line 9)\n" +
>+			"3. WARNING in X.java (at line 9)\n" +
> 			"	super(new A(){	\n" +
> 			"	          ^^^\n" +
> 			"Access to enclosing constructor A() is emulated by a synthetic accessor method\n" +
> 			"----------\n" +
>-			"5. WARNING in X.java (at line 10)\n" +
>+			"4. WARNING in X.java (at line 10)\n" +
> 			"	void foo() { System.out.println(X.this);	} \n" +
> 			"	     ^^^^^\n" +
> 			"The method foo() from the type new A(){} is never used locally\n" +
> 			"----------\n" +
>-			"6. ERROR in X.java (at line 10)\n" +
>+			"5. ERROR in X.java (at line 10)\n" +
> 			"	void foo() { System.out.println(X.this);	} \n" +
> 			"	                                ^^^^^^\n" +
> 			"No enclosing instance of the type X is accessible in scope\n" +
>@@ -3191,24 +3176,19 @@
> 			"	      ^\n" +
> 			"The type B is never used locally\n" +
> 			"----------\n" +
>-			"2. WARNING in X.java (at line 8)\n" +
>-			"	B() {	\n" +
>-			"	^^^\n" +
>-			"The constructor B() is never used locally\n" +
>-			"----------\n" +
>-			"3. ERROR in X.java (at line 9)\n" +
>+			"2. ERROR in X.java (at line 9)\n" +
> 			"	super(new A(){	\n" +
> 			"					void foo() { System.out.println(X.this);	} \n" +
> 			"				});	\n" +
> 			"	      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
> 			"No enclosing instance of type X is available due to some intermediate constructor invocation\n" +
> 			"----------\n" +
>-			"4. WARNING in X.java (at line 10)\n" +
>+			"3. WARNING in X.java (at line 10)\n" +
> 			"	void foo() { System.out.println(X.this);	} \n" +
> 			"	     ^^^^^\n" +
> 			"The method foo() from the type new A(){} is never used locally\n" +
> 			"----------\n" +
>-			"5. ERROR in X.java (at line 10)\n" +
>+			"4. ERROR in X.java (at line 10)\n" +
> 			"	void foo() { System.out.println(X.this);	} \n" +
> 			"	                                ^^^^^^\n" +
> 			"No enclosing instance of the type X is accessible in scope\n" +
>Index: src/org/eclipse/jdt/core/tests/compiler/regression/ProblemConstructorTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ProblemConstructorTest.java,v
>retrieving revision 1.22
>diff -u -r1.22 ProblemConstructorTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/ProblemConstructorTest.java	17 Feb 2009 12:35:51 -0000	1.22
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/ProblemConstructorTest.java	19 Feb 2009 11:06:40 -0000
>@@ -177,12 +177,17 @@
> 			"	           ^^^^^^^^^^^^^^\n" + 
> 			"The method unusedMethod() from the type X.M is never used locally\n" + 
> 			"----------\n" + 
>-			"2. WARNING in X.java (at line 6)\n" + 
>+			"2. WARNING in X.java (at line 5)\n" + 
>+			"	public M (int state) { this.state = state;} \n" + 
>+			"	       ^^^^^^^^^^^^^\n" + 
>+			"The constructor X.M(int) is never used locally\n" + 
>+			"----------\n" + 
>+			"3. WARNING in X.java (at line 6)\n" + 
> 			"	public int unusedField = 0;\n" + 
> 			"	           ^^^^^^^^^^^\n" + 
> 			"The field X.M.unusedField is never read locally\n" + 
> 			"----------\n" + 
>-			"3. WARNING in X.java (at line 7)\n" + 
>+			"4. WARNING in X.java (at line 7)\n" + 
> 			"	public class N {}\n" + 
> 			"	             ^\n" + 
> 			"The type X.M.N is never used locally\n" + 
>@@ -217,4 +222,104 @@
> 			"Access to enclosing method test() from the type A.B is emulated by a synthetic accessor method\n" + 
> 			"----------\n");
> }
>+//https://bugs.eclipse.org/bugs/show_bug.cgi?id=265142, wrong unused warning reported. Test to ensure that
>+//we DO complain about the constructor of B not being used when its base class has a no-arg constructor
>+public void test006() {
>+	this.runNegativeTest(
>+		new String[] {
>+				"A.java",
>+				"public class A {\n" +
>+			    "    public A(String s) {\n" +
>+			    "            B.test();\n" +
>+			    "    }\n" +
>+			    "    public A() {}\n" +
>+			    "\n" +
>+			    "    private static class B extends A {\n" +
>+			    "       public B () { super(\"\"); }\n" +
>+			    "\n" +
>+			    "            private static void test() {\n" +
>+			    "            };\n" +
>+			    "   }\n" +
>+				"}\n"
>+			},
>+			"----------\n" + 
>+			"1. WARNING in A.java (at line 3)\n" + 
>+			"	B.test();\n" + 
>+			"	^^^^^^^^\n" + 
>+			"Access to enclosing method test() from the type A.B is emulated by a synthetic accessor method\n" + 
>+			"----------\n" + 
>+			"2. WARNING in A.java (at line 8)\n" + 
>+			"	public B () { super(\"\"); }\n" + 
>+			"	       ^^^^\n" + 
>+			"The constructor A.B() is never used locally\n" + 
>+			"----------\n");
>+}
>+//https://bugs.eclipse.org/bugs/show_bug.cgi?id=265142, wrong unused warning reported. Test to ensure that
>+//we can compile the program successfully after deleting the unused constructor.
>+public void test007() {
>+	this.runConformTest(
>+		new String[] {
>+				"A.java",
>+				"public class A {\n" +
>+			    "    public A(String s) {\n" +
>+			    "            B.test();\n" +
>+			    "    }\n" +
>+			    "    public A() {}\n" +
>+			    "\n" +
>+			    "    private static class B extends A {\n" +
>+			    "            private static void test() {\n" +
>+			    "            };\n" +
>+			    "   }\n" +
>+				"}\n"
>+			});
>+}
>+//https://bugs.eclipse.org/bugs/show_bug.cgi?id=265142, wrong unused warning reported. Test to ensure that
>+//we DON'T complain about unused constructor when the super class's default constructor is not visible.
>+public void test008() {
>+	this.runNegativeTest(
>+		new String[] {
>+				"A.java",
>+				"public class A {\n" +
>+				" public A(String s) {}\n" +
>+				" public A() {}\n" +
>+				"}\n" +
>+				"class C {\n" +
>+				"	private static class B extends A {\n" +
>+				"		public B () { super(\"\"); }\n" +
>+				"       static void foo() {}\n" +
>+				"	}\n" +
>+				"	C() {\n" +
>+				"	B.foo();\n" +
>+				"	}\n" +
>+				"}\n"
>+				},
>+				"----------\n" + 
>+				"1. WARNING in A.java (at line 7)\n" + 
>+				"	public B () { super(\"\"); }\n" + 
>+				"	       ^^^^\n" + 
>+				"The constructor C.B() is never used locally\n" + 
>+				"----------\n");
>+}
>+//https://bugs.eclipse.org/bugs/show_bug.cgi?id=265142, wrong unused warning reported. Test to ensure that
>+//we DO complain about unused constructor when the super class's default constructor is visible.
>+public void test009() {
>+	this.runNegativeTest(
>+		new String[] {
>+				"A.java",
>+				"public class A {\n" +
>+				" public A(String s) {this();}\n" +
>+				" private A() {}\n" +
>+				"}\n" +
>+				"class C {\n" +
>+				"	private static class B extends A {\n" +
>+				"		public B () { super(\"\"); }\n" +
>+				"       static void foo() {}\n" +
>+				"	}\n" +
>+				"	C() {\n" +
>+				"	B.foo();\n" +
>+				"	}\n" +
>+				"}\n"
>+				},
>+				"");
>+}
> }

Flags:

kent_johnson: iplog+

Actions: View | Diff

Attachments on bug 265142: 126014 | 126152