Move enableNative to target, turn it on by default

pkg/front_end/lib/src/fasta/target_implementation.dart

 // Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library fasta.target_implementation;
 
 import 'package:kernel/target/targets.dart' as backend show Target;
 
 import 'builder/builder.dart' show Builder, ClassBuilder, LibraryBuilder;
 
+import 'source/source_library_builder.dart' show SourceLibraryBuilder;
+
 import 'parser/dart_vm_native.dart' as vm show skipNativeClause;
 
 import '../scanner/token.dart' show Token;
 
 import 'loader.dart' show Loader;
 
 import 'quote.dart' show unescapeString;
 
 import 'target.dart' show Target;
 
@@ skipping 60 matching lines @@
     LibraryBuilder internal = loader.read(Uri.parse("dart:_internal"), -1);
     return cachedNativeAnnotation = internal.getConstructor("ExternalName");
   }
 
   void loadExtraRequiredLibraries(Loader loader) {
     for (String uri in backendTarget.extraRequiredLibraries) {
       loader.read(Uri.parse(uri), -1);
     }
   }
 
+  /// Whether the `native` language extension is supported within [library].
+  ///
+  /// The `native` language extension is not part of the language specification,
+  /// means something else to each target, and is enabled differently for each
+  /// target implementation. For example, the VM target enables it everywhere
+  /// because of existing support for "dart-ext:" native extensions, but targets

[ahe, 2017/05/31 09:07:50]

I would really like to get this only enabled for dart-ext libraries. Could you
add a TODO mentioning this bug: https://github.com/dart-lang/sdk/issues/29763 ?

[Siggi Cherem (dart-lang), 2017/05/31 20:26:15]

Done.

+  /// like dart2js only enable it on the core libraries.
+  ///
+  /// This default implementation assumes a VM target, but it can be overriden
+  /// in subclasses to change the behavior.
+  bool enableNative(SourceLibraryBuilder library) => true;

[Siggi Cherem (dart-lang), 2017/05/30 20:33:59]

currently I use SourceLibraryBuilder because this is only relevant when dealing
with sources. Makes sense also that this is the first type where we define
`isPatch`.

[ahe, 2017/05/31 09:07:50]

I'm not sure the builder hierarchies make a lot of sense anymore, but using
SourceLibraryBuilder here goes against my original intent.

I'm also not convinced that "source" is precondition for "isPatch". I can easily
imagine scenarios where reading a patch file from a .dill file would make sense,
although such scenarios would be useful only to Dart platform library
developers.

Anyway, you don't need to change anything, but we do need to have a design
discussion about the builder hierarchies, and find some easy guidelines to
follow. Otherwise, we risk ending up in a similar situation as dart2js did
relying too much on AST nodes throughout its pipeline.

[Siggi Cherem (dart-lang), 2017/05/31 14:25:43]

I was hesitant too. The main reason I thought Source* made sense here, and not a
generic builder, was that I felt  "enableNative" was only relevant when we are
parsing and actually processing sources. That being said, I do like to keep APIs
simple and I would be happy adding isPatch to LibraryBuilder instead. I actually
did it locally, but didn't include in my CL.

Let me know if you prefer that at this time, and I'll include that change with
this CL.

[ahe, 2017/05/31 14:31:25]

That's up to you. An alternative is to do a type test when isPatch is needed.

[Siggi Cherem (dart-lang), 2017/05/31 20:26:14]

turns out that dart2js doesn't have any `native` in patch files either, so I
switchd back to LibraryBuilder and don't use isPatch anywhere.

+
   Token skipNativeClause(Token token) => vm.skipNativeClause(token);
 
   String extractNativeMethodName(Token token) =>
       unescapeString(token.next.lexeme);
 
   void addSourceInformation(
       Uri uri, List<int> lineStarts, List<int> sourceCode);
 
   void readPatchFiles(LibraryBuilder library) {
     assert(library.uri.scheme == "dart");
     List<Uri> patches = uriTranslator.patches[library.uri.path];
     if (patches != null) {
       for (Uri patch in patches) {
         library.loader.read(patch, -1, fileUri: patch, isPatch: true);
       }
     }
   }
 }