Create implementation file emitter in HtmlDartInterfaceGenerator.

Create implementation file emitter in HtmlDartInterfaceGenerator.

That way we can start emitting common parts in HtmlDartInterfaceGenerator.

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

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

[Anton Muhin, 2012-06-28 13:20:18]

Somewhat too late, but I'll still ask.

Now, when you have interface generator depend on single backend, what's the
right way to generate implementation for several backends?  I am not sure we
properly support this case even now, but it would look natural to be able to
generate interfaces only once.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
File lib/dom/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:793: def _FilePathForDartInterface(self,
interface_name):
does it still belong to HtmlInterfaceSystem, shouldn't it go to backends now?

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:807: self._system = system
do you need to store system here too (it's passed into super ctor?)

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:811: self._templates = system._templates
why are you caching _backend, _emitters, _shared and _templates?

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:815: interface_path =
self._system._FilePathForDartInterface(
it looks like interface_path is only used in then clause of conditional below,
may it go in there?

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:818:
self._system._dart_interface_file_paths.append(interface_path)
again, why you need _system here, or are you going to move things around bit by
bit?

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:827: implementation_path =
self._backend.FilePathForDartImplementation(
ditto for implementation_path and if below.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:830:
self._system._dart_interface_file_paths.append(implementation_path)
that looks slightly odd that you add implementation path into interface file
paths' list.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:837: if not IsPureInterface(self._interface.id):
that might be stupid, but it feels more natural to me to generate interface
first and then implementation.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:837: if not IsPureInterface(self._interface.id):
should you call into backend if it's not pure or if it
_backend.HasImplementation()?

[podivilov, 2012-06-29 12:59:28]

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
File lib/dom/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:793: def _FilePathForDartInterface(self,
interface_name):
On 2012/06/28 13:20:19, antonmuhin wrote:
> does it still belong to HtmlInterfaceSystem, shouldn't it go to backends now?

Dart interfaces are common in dartium and dart2js, so it shouldn't.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:807: self._system = system
On 2012/06/28 13:20:19, antonmuhin wrote:
> do you need to store system here too (it's passed into super ctor?)

No, it's not passed.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:811: self._templates = system._templates
On 2012/06/28 13:20:19, antonmuhin wrote:
> why are you caching _backend, _emitters, _shared and _templates?

Done.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:815: interface_path =
self._system._FilePathForDartInterface(
On 2012/06/28 13:20:19, antonmuhin wrote:
> it looks like interface_path is only used in then clause of conditional below,
> may it go in there?

Done.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:818:
self._system._dart_interface_file_paths.append(interface_path)
On 2012/06/28 13:20:19, antonmuhin wrote:
> again, why you need _system here, or are you going to move things around bit
by
> bit?

_dart_interface_file_paths is per-library state, let's keep it in system for
now.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:827: implementation_path =
self._backend.FilePathForDartImplementation(
On 2012/06/28 13:20:19, antonmuhin wrote:
> ditto for implementation_path and if below.

Done.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:830:
self._system._dart_interface_file_paths.append(implementation_path)
On 2012/06/28 13:20:19, antonmuhin wrote:
> that looks slightly odd that you add implementation path into interface file
> paths' list.

This is slightly complicated to refactor right now, because
_dart_interface_file_paths is used by SystemBase and shared with dom systems.
Let's keep for now.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:837: if not IsPureInterface(self._interface.id):
On 2012/06/28 13:20:19, antonmuhin wrote:
> that might be stupid, but it feels more natural to me to generate interface
> first and then implementation.

Done.

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:837: if not IsPureInterface(self._interface.id):
On 2012/06/28 13:20:19, antonmuhin wrote:
> should you call into backend if it's not pure or if it
> _backend.HasImplementation()?

Done.

[podivilov, 2012-06-29 13:05:17]

On 2012/06/28 13:20:18, antonmuhin wrote:
> Somewhat too late, but I'll still ask.
> 
> Now, when you have interface generator depend on single backend, what's the
> right way to generate implementation for several backends?  I am not sure we
> properly support this case even now, but it would look natural to be able to
> generate interfaces only once.
> 

The goal of this refactoring is to generate interfaces and implementation at
once, to eliminate common logic. So we'll need to put numerous loops all over
the code to support several backends at once. Personally, I don't think its
worth it.

[Anton Muhin, 2012-06-29 13:34:57]

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
File lib/dom/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10696033/diff/1003/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:807: self._system = system
Maybe you should pass it, but I am not insisting on it, and definitely not now.

On 2012/06/29 12:59:28, podivilov wrote:
> On 2012/06/28 13:20:19, antonmuhin wrote:
> > do you need to store system here too (it's passed into super ctor?)
> 
> No, it's not passed.

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
File lib/dom/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:810: self._backend =
system._backend.InterfaceGenerator(self._interface)
looks weird: interface generator assigned to backed.  Are you going to rename
this stuff later?

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:818: self._interface_emitter = emitter.Emitter()
why we use such emitters?  do I understand it correctly that was emitter emit
stuff into memory?  does anybody use the emitted code later?

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:905: if emit_events:
if not emit_events you continue to ln. 911 in new version, why it's ok?

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
File lib/dom/scripts/systemnative.py (right):

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
lib/dom/scripts/systemnative.py:210: self._cpp_header_emitter =
emitter.Emitter()
again, should we ever get here?  cannot you assert that
self.HasImplementation()?

[podivilov, 2012-06-29 13:59:41]

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
File lib/dom/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:810: self._backend =
system._backend.InterfaceGenerator(self._interface)
On 2012/06/29 13:34:57, antonmuhin wrote:
> looks weird: interface generator assigned to backed.  Are you going to rename
> this stuff later?

Renamed to ImplementationGenerator.

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:818: self._interface_emitter = emitter.Emitter()
On 2012/06/29 13:34:57, antonmuhin wrote:
> why we use such emitters?  do I understand it correctly that was emitter emit
> stuff into memory?  does anybody use the emitted code later?

This is a trick to avoid repeated checks for rare cases when either interface or
implementation shouldn't be generated. We always generate everything, but
unneeded files end up in memory.

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:905: if emit_events:
On 2012/06/29 13:34:57, antonmuhin wrote:
> if not emit_events you continue to ln. 911 in new version, why it's ok?

Yes, it was a bug actually.

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
File lib/dom/scripts/systemnative.py (right):

https://chromiumcodereview.appspot.com/10696033/diff/9001/lib/dom/scripts/sys...
lib/dom/scripts/systemnative.py:210: self._cpp_header_emitter =
emitter.Emitter()
On 2012/06/29 13:34:57, antonmuhin wrote:
> again, should we ever get here?  cannot you assert that
> self.HasImplementation()?

ditto.

[Anton Muhin, 2012-06-29 15:28:51]

lgtm