Use metadata annotation @patch for patch classes

Use metadata annotation @patch for patch classes

Annotate patch classes and top-level patch functions with @patch
instead of the pseudo-keyword patch. This allows the analyzer
to read patch files, and matches the syntax that dart2js uses.

The deprecated syntax is still supported, but a warning is printed when detected.


BUG=

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

[hausner, 2016-08-08 21:43:54]

Description was changed from

==========
Use metadata annotation @patch for patch classes

Annotate patch classes and top-level patch functions with @patch
instead of the pseudo-keyword patch.

The old syntax is still supported, but a warning is printed when
detected.

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

to

==========
Use metadata annotation @patch for patch classes

Annotate patch classes and top-level patch functions with @patch
instead of the pseudo-keyword patch. This allows the analyzer
to read patch files, and matches the syntax that dart2js uses.

The deprecated syntax is still supported, but a warning is printed when
detected.


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

[hausner, 2016-08-08 21:43:55]

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

[Vyacheslav Egorov (Google), 2016-08-09 11:31:06]

This looks very nice!

https://codereview.chromium.org/2220883004/diff/20001/runtime/bin/common_patc...
File runtime/bin/common_patch.dart (right):

https://codereview.chromium.org/2220883004/diff/20001/runtime/bin/common_patc...
runtime/bin/common_patch.dart:6: /* @patch */ static Uint8List
getRandomBytes(int count)
I wonder if could make it 

@patch static Uint8List getRandomBytes(int count)

?

The motivation would be to completely match dart2js patch files (it annotates
both classes and members).

This has an additional benefit. Imagine situation where we have

core.dart

class Foo {
  external void _foo();
}

core_patch.dart

class Foo {
  @patch void _foo();
}

and then somebody removes or renames _foo in core.dart.

In this case if we have @patch annotation we can actually detect if the member
we are patching is actually there.

[kasperl, 2016-08-09 11:48:08]

kasperl@google.com changed reviewers:
+ kasperl@google.com

[kasperl, 2016-08-09 11:48:09]

DBC

https://codereview.chromium.org/2220883004/diff/20001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2220883004/diff/20001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:4556: ReportWarning("deprecated use of patch annotation");
patch annotation -> patch "keyword"?

[hausner, 2016-08-09 15:48:45]

https://codereview.chromium.org/2220883004/diff/20001/runtime/bin/common_patc...
File runtime/bin/common_patch.dart (right):

https://codereview.chromium.org/2220883004/diff/20001/runtime/bin/common_patc...
runtime/bin/common_patch.dart:6: /* @patch */ static Uint8List
getRandomBytes(int count)
On 2016/08/09 11:31:06, Vyacheslav Egorov (Google) wrote:
> I wonder if could make it 
> 
> @patch static Uint8List getRandomBytes(int count)
> 
> ?
> 
> The motivation would be to completely match dart2js patch files (it annotates
> both classes and members).
> 
> This has an additional benefit. Imagine situation where we have
> 
> core.dart
> 
> class Foo {
>   external void _foo();
> }
> 
> core_patch.dart
> 
> class Foo {
>   @patch void _foo();
> }
> 
> and then somebody removes or renames _foo in core.dart.
> 
> In this case if we have @patch annotation we can actually detect if the member
> we are patching is actually there.

Yes, that would make sense. It's actually an old TODO from iposva.

Would it be fair to do this as a separate CL?

https://codereview.chromium.org/2220883004/diff/20001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2220883004/diff/20001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:4556: ReportWarning("deprecated use of patch annotation");
On 2016/08/09 11:48:09, kasperl wrote:
> patch annotation -> patch "keyword"?

Done. This legacy support and warning should go away within a week or two.

[Vyacheslav Egorov (Google), 2016-08-09 16:15:27]

https://codereview.chromium.org/2220883004/diff/20001/runtime/bin/common_patc...
File runtime/bin/common_patch.dart (right):

https://codereview.chromium.org/2220883004/diff/20001/runtime/bin/common_patc...
runtime/bin/common_patch.dart:6: /* @patch */ static Uint8List
getRandomBytes(int count)
> Would it be fair to do this as a separate CL?

SGTM!

[hausner, 2016-08-09 16:23:09]

Can I interpret that as an lgtm?

[hausner, 2016-08-09 16:48:09]

Description was changed from

==========
Use metadata annotation @patch for patch classes

Annotate patch classes and top-level patch functions with @patch
instead of the pseudo-keyword patch. This allows the analyzer
to read patch files, and matches the syntax that dart2js uses.

The deprecated syntax is still supported, but a warning is printed when
detected.


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

to

==========
Use metadata annotation @patch for patch classes

Annotate patch classes and top-level patch functions with @patch
instead of the pseudo-keyword patch. This allows the analyzer
to read patch files, and matches the syntax that dart2js uses.

The deprecated syntax is still supported, but a warning is printed when
detected.


BUG=

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

[hausner, 2016-08-09 16:48:10]

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

[Vyacheslav Egorov (Google), 2016-08-09 20:04:16]

LGTM