Fixes to patch files necessary to use the analyzer

Fixes to patch files necessary to use the analyzer

The analyzer has a stricter patch parser than the VM.  Patch files
cannot change signatures of patched members.  Specifically, they cannot
change:

  - the return type
  - a parameter's name
  - a parameter to an initializing formal
  - an optional parameter's default value

BUG=
R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/50348f5f22d8831cf11e92ff8548b6a6017b41cb

[Kevin Millikin (Google), 2017-01-04 15:31:04]

kmillikin@google.com changed reviewers:
+ asiva@google.com, vegorov@google.com

[Kevin Millikin (Google), 2017-01-04 15:31:05]

https://codereview.chromium.org/2612043002/diff/1/runtime/bin/file_patch.dart
File runtime/bin/file_patch.dart (right):

https://codereview.chromium.org/2612043002/diff/1/runtime/bin/file_patch.dart...
runtime/bin/file_patch.dart:103: Stream get _stream =>
_broadcastController.stream;
Patch files can't introduce new public members, so this one is made private
(like all the other members introduced in this class).

1. It might be possible to relax the restriction, so that patch files can add
public members to private classes.  That doesn't seem worth it for this single
use case.

2. This whole class has two public static members which have one call site each,
and both throw because they're unsupported on dart2js.  I think it would be
possible to patch the functions that call this class's members instead, and move
this entire class into the patch file (in which case you could introduce all the
public members you wanted).

https://codereview.chromium.org/2612043002/diff/1/runtime/lib/array_patch.dart
File runtime/lib/array_patch.dart (right):

https://codereview.chromium.org/2612043002/diff/1/runtime/lib/array_patch.dar...
runtime/lib/array_patch.dart:14: @patch factory List([int length]) =
List<E>._internal;
Patch files can't change the default value (it's null in the SDK).  That's
reasonable, otherwise a developer would get a different default than the API
docs say they should.  (This function is kind of magical regarding the
parameter, unfortunately).

The indirection costs nothing to Kernel because it is short-circuited.

https://codereview.chromium.org/2612043002/diff/1/runtime/lib/mirrors.cc
File runtime/lib/mirrors.cc (right):

https://codereview.chromium.org/2612043002/diff/1/runtime/lib/mirrors.cc#newc...
runtime/lib/mirrors.cc:1670: if
(!redirected_constructor.AreValidArguments(args_descriptor, NULL)) {
We have already checked above (line 1600) that lookup_constructor --- the one
we're actually trying to invoke --- is reflectable.  I think it should not
matter if the redirection target is or is not.

Private members of SDK libraries are not reflectable.  If this check is left
here, it prevents mirrors from invoking the factory constructor List (which is
reflectable) because it redirects to List._internal (which is not reflectable).

I'm less sure about the NSM error message.  I've kept it the same, but it will
be in terms of the redirected constructor.

https://codereview.chromium.org/2612043002/diff/1/sdk/lib/_internal/js_runtim...
File sdk/lib/_internal/js_runtime/lib/developer_patch.dart (right):

https://codereview.chromium.org/2612043002/diff/1/sdk/lib/_internal/js_runtim...
sdk/lib/_internal/js_runtime/lib/developer_patch.dart:49: void _postEvent(String
eventKind, String eventData) {
Patch files cannot change the return type.  In this case, the SDK's return type
is omitted (i.e., dynamic) and the VM's return type is void.

I've just changed the SDK's return type to be void too, because it's better.

https://codereview.chromium.org/2612043002/diff/1/sdk/lib/_internal/js_runtim...
sdk/lib/_internal/js_runtime/lib/developer_patch.dart:143: class _FakeUserTag
implements UserTag {
This implementation machinery was in the SDK and ignored by the VM (it patched
members that were not declared external, which isn't actually allowed).

I've moved it to the dart2js patch here and declared the members external.

[siva, 2017-01-05 01:08:00]

All changes to files in runtime/bin and runtime/lib and runtime/vm (The VM part)
LGTM.

I am not very familiar with the dart2js patch files maybe somebody from that
team could look at those files.

[Kevin Millikin (Google), 2017-01-05 11:23:59]

Description was changed from

==========
Fixes to patch files necessary to use the analyzer

The analyzer has a stricter patch parser than the VM.  Patch files
cannot change signatures of patched members.  Specifically, they cannot
change:

  - the return type
  - a parameter's name
  - a parameter to an initializing formal
  - an optional parameter's default value

BUG=
==========

to

==========
Fixes to patch files necessary to use the analyzer

The analyzer has a stricter patch parser than the VM.  Patch files
cannot change signatures of patched members.  Specifically, they cannot
change:

  - the return type
  - a parameter's name
  - a parameter to an initializing formal
  - an optional parameter's default value

BUG=
R=asiva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/50348f5f22d8831cf11e92ff8548b6a6017b41cb
==========

[Kevin Millikin (Google), 2017-01-05 11:24:01]

Committed patchset #1 (id:1) manually as
50348f5f22d8831cf11e92ff8548b6a6017b41cb (presubmit successful).