Do not merge IDL interfaces in dartdomgenerator.

Do not merge IDL interfaces in dartdomgenerator.

The merge is irreversible, and shouldn't be done too early.

R=antonm@google.com,sra@google.com

Committed: https://code.google.com/p/dart/source/detail?r=8456

[Anton Muhin, 2012-06-06 16:14:19]

Sorry, I leave it for Stephen to comment on this issue.

[sra1, 2012-06-06 21:15:53]

I am worried about two things:

1. The old merge will collapse identical definitions coming from both sources,
but the new code will end up with both.

2. The form of definitions in the subclass depends on the exact form of the
members in all of the superclasses.  This could be wrong because the lookup of
the superclasses finds the unmerged class and gets the wrong idea about
overriding.

I see html_dartium might just have rearranged the definitinions:
 +53 lines, -53 lines = +0 lines
This is what we would expect.

html_frog:
 +91 lines, -97 lines = -6 lines?
I suspect this indicates that there is at least one of the problems I am worried
about.

[podivilov, 2012-06-07 07:56:23]

On 2012/06/06 21:15:53, sra1 wrote:
> I am worried about two things:
> 
> 1. The old merge will collapse identical definitions coming from both sources,
> but the new code will end up with both.

The only two merged interfaces were HTMLDocument->Document,
HTMLElement->Element.
Fortunately, there is no such cases at all, and both interfaces are very stable.

> 
> 2. The form of definitions in the subclass depends on the exact form of the
> members in all of the superclasses.  This could be wrong because the lookup of
> the superclasses finds the unmerged class and gets the wrong idea about
> overriding.
> 
> I see html_dartium might just have rearranged the definitinions:
>  +53 lines, -53 lines = +0 lines
> This is what we would expect.
> 
> html_frog:
>  +91 lines, -97 lines = -6 lines?
> I suspect this indicates that there is at least one of the problems I am
worried
> about.

Fixed overriding logic to take merging into account.

[sra1, 2012-06-07 19:31:07]

There is a lot of stuff here that works only because of the current definitions
of the interfaces.  We know that the interfaces are 'stable' but they are only
stable until they are not.

I'm OK with proceeding along this path, but only if the assumptions are well
documented.

Document the issues I'll make further comments and/or lgtm.
Check it in 'TBR' if that helps you make progress.

http://codereview.chromium.org/10541029/diff/9001/lib/dom/scripts/systemfrog.py
File lib/dom/scripts/systemfrog.py (right):

http://codereview.chromium.org/10541029/diff/9001/lib/dom/scripts/systemfrog....
lib/dom/scripts/systemfrog.py:301: return FindInParent(parent_interface)
If parent_interface was merged into some other interface, we should visit the
interface it was merged into, otherwise we visit HTMLElement twice.

It does not matter here, but only because we don't currently do much with the
superinterface.

Please either (a) fix the code so it does the 'correct' search or add a comment
here explaining why the incorrect search is OK.

http://codereview.chromium.org/10541029/diff/9001/lib/dom/scripts/systemhtml.py
File lib/dom/scripts/systemhtml.py (right):

http://codereview.chromium.org/10541029/diff/9001/lib/dom/scripts/systemhtml....
lib/dom/scripts/systemhtml.py:264: _merged_html_interfaces = {
This needs an essay.

Add comment what this means (which gets merged with which).

Explain the consequences - that SVGElement has new 'HTML' methods that need to
be implemented, that Element itself (which is used for XML elements) also has
new methods that need to be implemented.

Explain that the mechanism is not very general, and following restrictions
apply:

(a) only works for 'compressing out' an interface on the primary
(implementation) inheritance chain.

(b) only works because the two interfaces that are merged have no members in
common.

[podivilov, 2012-06-08 17:02:21]

PTAL.

http://codereview.chromium.org/10541029/diff/9001/lib/dom/scripts/systemfrog.py
File lib/dom/scripts/systemfrog.py (right):

http://codereview.chromium.org/10541029/diff/9001/lib/dom/scripts/systemfrog....
lib/dom/scripts/systemfrog.py:301: return FindInParent(parent_interface)
On 2012/06/07 19:31:07, sra1 wrote:
> If parent_interface was merged into some other interface, we should visit the
> interface it was merged into, otherwise we visit HTMLElement twice.
> 
> It does not matter here, but only because we don't currently do much with the
> superinterface.
> 
> Please either (a) fix the code so it does the 'correct' search or add a
comment
> here explaining why the incorrect search is OK.

Fixed.

http://codereview.chromium.org/10541029/diff/9001/lib/dom/scripts/systemhtml.py
File lib/dom/scripts/systemhtml.py (right):

http://codereview.chromium.org/10541029/diff/9001/lib/dom/scripts/systemhtml....
lib/dom/scripts/systemhtml.py:264: _merged_html_interfaces = {
On 2012/06/07 19:31:07, sra1 wrote:
> This needs an essay.
> 
> Add comment what this means (which gets merged with which).
> 
> Explain the consequences - that SVGElement has new 'HTML' methods that need to
> be implemented, that Element itself (which is used for XML elements) also has
> new methods that need to be implemented.
> 
> Explain that the mechanism is not very general, and following restrictions
> apply:
> 
> (a) only works for 'compressing out' an interface on the primary
> (implementation) inheritance chain.
> 
> (b) only works because the two interfaces that are merged have no members in
> common.

Done.

[sra1, 2012-06-08 17:24:51]

lgtm