Bug 146768

Summary:

[compiler] Should be more resilient with duplicate fields/methods

Product:

[Eclipse Project] JDT

Reporter:

Philipe Mulet <philippe_mulet>

Component:

Core

Assignee:

Kent Johnson <kent_johnson>

Status:

VERIFIED FIXED

QA Contact:

Severity:

enhancement

Priority:

CC:

david_audel, markus.kell.r

Version:

3.2

Target Milestone:

3.5 M3

Hardware:

OS:

Windows XP

Whiteboard:

Bug Depends on:

Bug Blocks:

156731

Attachments:

Description	Flags
Proposed patch with testcase for duplicate fields	none
Proposed patch with testcases	none
Proposed patch with testcases	none

Description Philipe Mulet

2006-06-13 03:27:44 EDT

Build 3.2RC7

Along the lines of bug 144858.

In presence of duplicate fields or methods; the
compiler discards all instances of the offending construct.
This means that more secondary errors may arise and be quite misleading outside
of context, and other tools based on the AST (DOM AST for instance) may behave
suboptimally as lacking some of the binding information.

What if:
- it still created the bindings, and attached them to the AST node
- it would leave at least one entry in binding arrays to reduce amount of
secondary errors.

e.g. we currently complain about missing both #foo() and #baz()
public class X {
        void foo() {
        }
        void foo() {
        }
        void bar() {
                foo();
                baz();
        }
}

Comment 1 Jerome Lanneluc

2007-03-22 06:15:30 EDT

To be investigated during M7

Comment 2 Philipe Mulet

2007-03-28 07:35:07 EDT

Nice to have. I wouldn't change this at this point in the release cycle.
Removing 3.3M7 target

Comment 3 Kent Johnson

2008-10-03 16:29:28 EDT

From this testcase, we delete both duplicate fields & methods (but we keep the first duplicate superinterface, member type & thrown exception) :

class X<T> implements I, I {
	int i;
	boolean i;
	void bar() {
		M m = new M();
		m.foo();
		if (i) {
			try { test(); } catch ( Ex e ) {}
		}
	}
	public void test() throws Ex, Ex {} // do not complain about extra Ex
	class M {
		void foo() {}
		void foo() {}
	}
	class M {}
}
interface I {
	void test() throws Ex, Ex; // do not complain about extra Ex
}
@SuppressWarnings("serial")
class Ex extends Exception {}

Comment 4 Kent Johnson

2008-10-14 15:52:40 EDT

Created attachment 115077 [details]
Proposed patch with testcase for duplicate fields

This patch keeps the first duplicate field & removes the rest

Comment 5 Kent Johnson

2008-10-16 15:35:00 EDT

Created attachment 115301 [details]
Proposed patch with testcases

Comment 6 Kent Johnson

2008-10-17 14:05:34 EDT

Fix released for 3.5M3

Updated tests in 8 different test classes including NegativeTest and MethodVerifyTest

Comment 7 Kent Johnson

2008-10-21 13:51:45 EDT

Created attachment 115727 [details]
Proposed patch with testcases

Releasing this patch instead.

Both duplicate fields and methods will receive errors but the binding for the first field/method will still exist to reduce secondary errors.

Comment 8 David Audel

2008-10-28 09:04:00 EDT

Verified for 3.5M3 using I20081026-2000 build.

Comment 9 Kent Johnson

2008-11-14 14:26:55 EST

See bug 255035 for additional change when a duplicate method did not have a return type