Move enableNative to target, turn it on by default

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

Index: pkg/front_end/lib/src/fasta/target_implementation.dart
diff --git a/pkg/front_end/lib/src/fasta/target_implementation.dart b/pkg/front_end/lib/src/fasta/target_implementation.dart
index 708795af65bb2c1722f76b4e9d4e9c4b58a04487..c2926f6bf291d2fa186a8ee3caf66f5bdf49525e 100644
--- a/pkg/front_end/lib/src/fasta/target_implementation.dart
+++ b/pkg/front_end/lib/src/fasta/target_implementation.dart
@@ -8,6 +8,8 @@ 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;
@@ -88,6 +90,18 @@ abstract class TargetImplementation extends Target {
     }
   }
 
+  /// 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) =>