Add @JSName annotation for native fields and methods.

Add @JSName annotation for native fields and methods.

This change allows fields to have different Dart and JavaScript names
without resorting to getters and setters using the 'JS' call.

For uniformity, methods have been changed use @JSName too.  The
previous scheme of using a string containing an identifier following
the 'native' pseudo-keyword is now an error.  A non-identifier string,
giving a native body to a Dart function, is still supported.

Members are marked as native during resolution, making the
Element.isNative() predicate more reliable.

Some uses of nativeName have been renamed to nativeTagInfo or
nativeTag to reflect that the strings are actually the class names.

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

[sra1, 2012-11-27 23:54:28]

Nicolas: please review the compiler stuff.
Pete: the generator and generated results.

[ngeoffray, 2012-11-28 20:31:23]

LGTM! It looks like you could also remove the 'native' suffix on the declaration
of these fields/methods. You would treat all @JSName implicitely making them
native.

https://codereview.chromium.org/11299220/diff/5001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11299220/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:496: if (libraryName ==
'dart:html' ||
Please keep the style used lines 491-495. Otherwise the code loos very strange.

https://codereview.chromium.org/11299220/diff/5001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/11299220/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:782: :
accessorName;
fits in one line now?

https://codereview.chromium.org/11299220/diff/5001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/native_handler.dart (right):

https://codereview.chromium.org/11299220/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/native_handler.dart:174:
PartialMetadataAnnotation partial = annotation;
Do you need this cast just because of parseNode? Why isn't parseNode on
MetadataAnnotation?

https://codereview.chromium.org/11299220/diff/5001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/11299220/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:1725: String name =
node.element.isNative()
Move this in the namer? It would avoid the code duplication with line 1739.

[blois, 2012-11-28 21:50:55]

lgtm

https://codereview.chromium.org/11299220/diff/1/sdk/lib/html/scripts/systemht...
File sdk/lib/html/scripts/systemhtml.py (right):

https://codereview.chromium.org/11299220/diff/1/sdk/lib/html/scripts/systemht...
sdk/lib/html/scripts/systemhtml.py:513: #if attribute.id != html_name:
Remove?

[ngeoffray, 2012-11-29 08:40:54]

+ahe for the annotation parsing

[ahe, 2012-11-29 09:19:26]

DBC

https://codereview.chromium.org/11299220/diff/4004/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/11299220/diff/4004/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:358: String
_nativeName = null;
Please follow this layout of class definitions:

fields
constructors
methods

https://codereview.chromium.org/11299220/diff/4004/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11299220/diff/4004/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:560: subarray =
JS('=List', r'array.slice(#, #)',
How about adding a new foreign tag that clearly communicates what = means.

https://codereview.chromium.org/11299220/diff/4004/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/native_handler.dart (right):

https://codereview.chromium.org/11299220/diff/4004/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/native_handler.dart:159: String
findJsNameFromAnnotation(Element element) {
This file should be deleted when we get rid of the native keyword.  The purpose
of this file is to isolate support for the legacy support for the frog native
keyword feature.

I assume this method should survive the legacy support for native keyword, so it
probably doesn't belong in this file.

https://codereview.chromium.org/11299220/diff/4004/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/native_handler.dart:165:
MetadataAnnotation annotation = link.head.ensureResolved(compiler);
I'm think we should make this an operation on element.  For example:

Link<MetadataAnnotation> metadata = element.getResolvedMetadata(compiler);

https://codereview.chromium.org/11299220/diff/4004/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/native_handler.dart:172: // TODO(sra):
Better validation of the constant.
The constant will be validated in checked mode.  We could change the constant
evaluator to warn about things that it knows will fail in checked mode.

https://codereview.chromium.org/11299220/diff/4004/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/native_handler.dart:176: 'Annotations
needs one string: ${partial.parseNode(compiler)}');
This should be:

compiler.internalError('Annotations needs one string', node:
partial.parseNode(compiler));

But I'll make MetadataAnnotation implement Spannable so it is easier to use for
a location.

https://codereview.chromium.org/11299220/diff/4004/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/native_handler.dart:184: 'Too many
JSName annotations: ${partial.parseNode(compiler)}');
Ditto.

[erikcorry, 2012-11-29 12:55:21]

I'm sorry I seem to have sabotaged this with my rollback.

I tried putting it on top of a new roll-forward, but it seems like the tests
have not been updated.  For example:

tests/compiler/dart2js_native/native_field_rename_2_frog_test.dart:25:34:
Deprecated syntax, use @JSName('name') instead.
  int native_key_method() native 'key';
                                 ^^^^^
Error: Compilation failed.

[sra1, 2012-12-04 01:31:31]

https://chromiumcodereview.appspot.com/11299220/diff/5001/sdk/lib/_internal/c...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://chromiumcodereview.appspot.com/11299220/diff/5001/sdk/lib/_internal/c...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:782: :
accessorName;
On 2012/11/28 20:31:23, ngeoffray wrote:
> fits in one line now?

Not quite.

https://chromiumcodereview.appspot.com/11299220/diff/5001/sdk/lib/_internal/c...
File sdk/lib/_internal/compiler/implementation/native_handler.dart (right):

https://chromiumcodereview.appspot.com/11299220/diff/5001/sdk/lib/_internal/c...
sdk/lib/_internal/compiler/implementation/native_handler.dart:174:
PartialMetadataAnnotation partial = annotation;
On 2012/11/28 20:31:23, ngeoffray wrote:
> Do you need this cast just because of parseNode? Why isn't parseNode on
> MetadataAnnotation?

I don't know, but MetadataAnnotation is more 'interface-like' than the others. 
I've opened Issue 7007.

https://chromiumcodereview.appspot.com/11299220/diff/4004/sdk/lib/_internal/c...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/11299220/diff/4004/sdk/lib/_internal/c...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:358: String
_nativeName = null;
On 2012/11/29 09:19:26, ahe wrote:
> Please follow this layout of class definitions:
> 
> fields
> constructors
> methods

Sure, but (1) I was following existing style (2) what exactly is improved by
moving the field 220 lines (several pages) away?

https://chromiumcodereview.appspot.com/11299220/diff/4004/sdk/lib/_internal/c...
File sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://chromiumcodereview.appspot.com/11299220/diff/4004/sdk/lib/_internal/c...
sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:560: subarray =
JS('=List', r'array.slice(#, #)',
On 2012/11/29 09:19:26, ahe wrote:
> How about adding a new foreign tag that clearly communicates what = means.

Do you mean another thing like 'JS'?
I don't think that scales, since the type is orthogonal to the JS text, and the
syntax is shared with @Returns and @Creates annotations on native methods.

We need to be able to tell the compiler types that are not expressible in Dart,
otherwise we can't tree-shake.  So =List needs to compose with other types, e.g.
'=List|Null' is the nullable type, 'ArrayBuffer|Document|String' is (approx)
what you get from an xhr response.

I thought about using type literals and combinators but it was verbose, ugly,
and difficult use, e.g.

  JS(const Union(const Exactly(List), Null), "...", x, x, x)

https://chromiumcodereview.appspot.com/11299220/diff/4004/sdk/lib/_internal/c...
File sdk/lib/_internal/compiler/implementation/native_handler.dart (right):

https://chromiumcodereview.appspot.com/11299220/diff/4004/sdk/lib/_internal/c...
sdk/lib/_internal/compiler/implementation/native_handler.dart:176: 'Annotations
needs one string: ${partial.parseNode(compiler)}');
On 2012/11/29 09:19:26, ahe wrote:
> This should be:
> 
> compiler.internalError('Annotations needs one string', node:
> partial.parseNode(compiler));
> 
> But I'll make MetadataAnnotation implement Spannable so it is easier to use
for
> a location.

This would be sufficient for Issue 7007.

[erikcorry, 2012-12-04 09:33:26]

This was reverted, the new version is https://codereview.chromium.org/11419250/