Serialize also canonical names of 'interfaceTargetReference'.

Serialize also canonical names of 'interfaceTargetReference'.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/8460f770b1570259fc5fc4aeeaa4a700019426fd

[Siggi Cherem (dart-lang), 2017-05-25 18:58:34]

Kevin - can you take a look?

While the changes seem fine to me, I don't know what is the right kernel
invariant here. This relates to a similar question I sent on email last week: is
targetReference/interfaceTargetReference always going to be filled? Peter
suggested that maybe that was only when we had strong mode enabled?

[ahe, 2017-05-29 13:51:51]

I took a look, and I think I would prefer if Kevin reviews this.

I'm a bit concerned with the extension of the visitor pattern, and think it
might be better to limit this extension to LimitedBinaryPrinter or BinaryPrinter
until we have a clear idea that this generally belongs in the visitor API.

[Kevin Millikin (Google), 2017-05-30 15:11:34]

I have a suggestion for the visitor API.  LGTM if you address the suggestion.

https://codereview.chromium.org/2906693002/diff/1/pkg/kernel/lib/ast.dart
File pkg/kernel/lib/ast.dart (right):

https://codereview.chromium.org/2906693002/diff/1/pkg/kernel/lib/ast.dart#new...
pkg/kernel/lib/ast.dart:1697:
v.visitInterfaceTargetReference(interfaceTargetReference);
We don't usually structure visitors based on the semantics (purpose) of a
subpart, but rather the syntax (type) of it.  For instance, we wouldn't have
visitLoopCondition and visitLoopBody, but visitExpression and visitStatement.

I suggest to eliminate visitInterfaceTargetReference for this change and call
instead:

interfaceTargetReference?.acceptReference(v);

here and below.

[scheglov, 2017-05-30 17:45:50]

https://codereview.chromium.org/2906693002/diff/1/pkg/kernel/lib/ast.dart
File pkg/kernel/lib/ast.dart (right):

https://codereview.chromium.org/2906693002/diff/1/pkg/kernel/lib/ast.dart#new...
pkg/kernel/lib/ast.dart:1697:
v.visitInterfaceTargetReference(interfaceTargetReference);
On 2017/05/30 15:11:33, Kevin Millikin (Google) wrote:
> We don't usually structure visitors based on the semantics (purpose) of a
> subpart, but rather the syntax (type) of it.  For instance, we wouldn't have
> visitLoopCondition and visitLoopBody, but visitExpression and visitStatement.
> 
> I suggest to eliminate visitInterfaceTargetReference for this change and call
> instead:
> 
> interfaceTargetReference?.acceptReference(v);
> 
> here and below.

Hm... There is no method acceptReference in Reference.

So, I will go with Peter's suggestion and update ReferencesStringIndexer to
process nodes that have interfaceTargetReference instead of changing AST and
visitors.

[scheglov, 2017-05-30 17:54:20]

Description was changed from

==========
Serialize also canonical names of 'interfaceTargetReference'.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=
==========

to

==========
Serialize also canonical names of 'interfaceTargetReference'.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/8460f770b1570259fc5fc4aeeaa4a700019426fd
==========

[scheglov, 2017-05-30 17:54:21]

Committed patchset #2 (id:20001) manually as
8460f770b1570259fc5fc4aeeaa4a700019426fd (presubmit successful).