From 05f3b12960fbe1808d367df18b86095b1baed369 Mon Sep 17 00:00:00 2001 From: Johnni Winther Date: Thu, 28 Mar 2019 10:29:11 +0000 Subject: [PATCH] Add test for native declarations Change-Id: Iea9d8d27e6fdefa6f47471a459cdb4363aab65e1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98011 Commit-Queue: Johnni Winther Reviewed-by: Sigmund Cherem --- .../lib/src/diagnostics/messages.dart | 5 + .../lib/src/kernel/dart2js_target.dart | 3 +- .../lib/src/kernel/element_map_impl.dart | 25 ++- .../dart2js/inlining/data/external.dart | 6 + tests/compiler/dart2js/model/native_test.dart | 66 +++++- .../compiler/dart2js_native/native_test.dart | 211 ++++++++++++++++++ third_party/googletest | 1 + 7 files changed, 311 insertions(+), 6 deletions(-) create mode 100644 tests/compiler/dart2js_native/native_test.dart create mode 160000 third_party/googletest diff --git a/pkg/compiler/lib/src/diagnostics/messages.dart b/pkg/compiler/lib/src/diagnostics/messages.dart index 16aee2168138..d04410fd49a0 100644 --- a/pkg/compiler/lib/src/diagnostics/messages.dart +++ b/pkg/compiler/lib/src/diagnostics/messages.dart @@ -88,6 +88,7 @@ enum MessageKind { LIBRARY_NOT_FOUND, MIRRORS_LIBRARY_NOT_SUPPORT_WITH_CFE, MISSING_EXPRESSION_IN_THROW, + NATIVE_NON_INSTANCE_IN_NON_NATIVE_CLASS, NO_SUCH_SUPER_MEMBER, NON_NATIVE_EXTERNAL, NOT_A_COMPILE_TIME_CONSTANT, @@ -677,6 +678,10 @@ become a compile-time error in the future."""), "Try removing 'external' keyword or annotating the function " "as a js-interop function."), + MessageKind.NATIVE_NON_INSTANCE_IN_NON_NATIVE_CLASS: const MessageTemplate( + MessageKind.NATIVE_NON_INSTANCE_IN_NON_NATIVE_CLASS, + "Native non-instance members are only allowed in native classes."), + // TODO(32557): Remove these when issue 32557 is fixed. MessageKind.SWITCH_CASE_VALUE_OVERRIDES_EQUALS: const MessageTemplate( MessageKind.SWITCH_CASE_VALUE_OVERRIDES_EQUALS, diff --git a/pkg/compiler/lib/src/kernel/dart2js_target.dart b/pkg/compiler/lib/src/kernel/dart2js_target.dart index 29361f884754..3d32f5f8b1b9 100644 --- a/pkg/compiler/lib/src/kernel/dart2js_target.dart +++ b/pkg/compiler/lib/src/kernel/dart2js_target.dart @@ -30,7 +30,8 @@ bool maybeEnableNative(Uri uri) { bool allowedTestLibrary() { String scriptName = uri.path; return scriptName.contains('tests/compiler/dart2js_native') || - scriptName.contains('tests/compiler/dart2js_extra'); + scriptName.contains('tests/compiler/dart2js_extra') || + scriptName.contains('generated_tests/dart2js_native/native_test'); } bool allowedDartLibrary() { diff --git a/pkg/compiler/lib/src/kernel/element_map_impl.dart b/pkg/compiler/lib/src/kernel/element_map_impl.dart index 7868bfc1ac80..072edd14fd12 100644 --- a/pkg/compiler/lib/src/kernel/element_map_impl.dart +++ b/pkg/compiler/lib/src/kernel/element_map_impl.dart @@ -2035,6 +2035,13 @@ class KernelNativeMemberResolver implements NativeMemberResolver { ir.Member node, IrAnnotationData annotationData) { if (_isNativeMethod(node, annotationData)) { if (node.enclosingClass != null && !node.isInstanceMember) { + if (!_nativeBasicData + .isNativeClass(_elementMap.getClass(node.enclosingClass))) { + _elementMap.reporter.reportErrorMessage( + computeSourceSpanFromTreeNode(node), + MessageKind.NATIVE_NON_INSTANCE_IN_NON_NATIVE_CLASS); + return false; + } _setNativeNameForStaticMethod(node, annotationData); } else { _setNativeName(node, annotationData); @@ -2065,8 +2072,8 @@ class KernelNativeMemberResolver implements NativeMemberResolver { String name = _findJsNameFromAnnotation(node, annotationData); name ??= node.name.name; if (_isIdentifier(name)) { - List nativeNames = _nativeBasicData - .getNativeTagsOfClass(_elementMap.getClass(node.enclosingClass)); + ClassEntity cls = _elementMap.getClass(node.enclosingClass); + List nativeNames = _nativeBasicData.getNativeTagsOfClass(cls); if (nativeNames.length != 1) { failedAt( computeSourceSpanFromTreeNode(node), @@ -2157,15 +2164,25 @@ class KernelNativeMemberResolver implements NativeMemberResolver { bool _isNativeMethod(ir.Member node, IrAnnotationData annotationData) { if (!maybeEnableNative(node.enclosingLibrary.importUri)) return false; + bool hasNativeBody; if (annotationData != null) { - return annotationData.hasNativeBody(node); + hasNativeBody = annotationData.hasNativeBody(node); } else { - return node.annotations.any((ir.Expression expression) { + hasNativeBody = node.annotations.any((ir.Expression expression) { return expression is ir.ConstructorInvocation && _elementMap.getInterfaceType(expression.constructedType) == _commonElements.externalNameType; }); } + if (!hasNativeBody && + node.isExternal && + !_nativeBasicData.isJsInteropMember(_elementMap.getMember(node))) { + // TODO(johnniwinther): Should we change dart:html and friends to use + // `external` instead of the native body syntax? + _elementMap.reporter.reportErrorMessage( + computeSourceSpanFromTreeNode(node), MessageKind.NON_NATIVE_EXTERNAL); + } + return hasNativeBody; } bool _isJsInteropMember(ir.Member node) { diff --git a/tests/compiler/dart2js/inlining/data/external.dart b/tests/compiler/dart2js/inlining/data/external.dart index 2680f316ef4a..b656ff442a3b 100644 --- a/tests/compiler/dart2js/inlining/data/external.dart +++ b/tests/compiler/dart2js/inlining/data/external.dart @@ -2,6 +2,11 @@ // 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. +@JS() +library lib; + +import 'package:js/js.dart'; + /*element: main:[]*/ main() { externalFunction(); @@ -14,4 +19,5 @@ externalFunction() { } /*element: _externalFunction:[]*/ +@JS('externalFunction') external _externalFunction(); diff --git a/tests/compiler/dart2js/model/native_test.dart b/tests/compiler/dart2js/model/native_test.dart index c281c63f889c..6ae4e3235fb4 100644 --- a/tests/compiler/dart2js/model/native_test.dart +++ b/tests/compiler/dart2js/model/native_test.dart @@ -139,7 +139,71 @@ main() { '53', '54', ]); - // TODO(johnniwinther): Add similar test for native declarations. + + await runTest('tests/compiler/dart2js_native/native_test.dart', + 'tests/compiler/dart2js_native/', { + 'Class': Kind.regular, + 'NativeClass': Kind.native, + 'topLevelField': Kind.regular, + 'topLevelGetter': Kind.regular, + 'topLevelSetter': Kind.regular, + 'topLevelFunction': Kind.regular, + 'nativeTopLevelGetter': Kind.native, + 'nativeTopLevelSetter': Kind.native, + 'nativeTopLevelFunction': Kind.native, + 'Class.generative': Kind.regular, + 'Class.fact': Kind.regular, + 'Class.instanceField': Kind.regular, + 'Class.instanceGetter': Kind.regular, + 'Class.instanceSetter': Kind.regular, + 'Class.instanceMethod': Kind.regular, + 'Class.staticField': Kind.regular, + 'Class.staticGetter': Kind.regular, + 'Class.staticSetter': Kind.regular, + 'Class.staticMethod': Kind.regular, + 'Class.nativeInstanceGetter': Kind.native, + 'Class.nativeInstanceSetter': Kind.native, + 'Class.nativeInstanceMethod': Kind.native, + 'NativeClass.generative': Kind.regular, + 'NativeClass.fact': Kind.regular, + 'NativeClass.nativeGenerative': Kind.native, + 'NativeClass.nativeFact': Kind.native, + 'NativeClass.instanceField': Kind.native, + 'NativeClass.instanceGetter': Kind.regular, + 'NativeClass.instanceSetter': Kind.regular, + 'NativeClass.instanceMethod': Kind.regular, + 'NativeClass.staticField': Kind.regular, + 'NativeClass.staticGetter': Kind.regular, + 'NativeClass.staticSetter': Kind.regular, + 'NativeClass.staticMethod': Kind.regular, + 'NativeClass.nativeInstanceGetter': Kind.native, + 'NativeClass.nativeInstanceSetter': Kind.native, + 'NativeClass.nativeInstanceMethod': Kind.native, + 'NativeClass.nativeStaticGetter': Kind.native, + 'NativeClass.nativeStaticSetter': Kind.native, + 'NativeClass.nativeStaticMethod': Kind.native, + }, + skipList: [ + // External constructors in non-native class + //'08', + //'09', + // External instance members in non-native class + //'22', + //'23', + //'24', + // External static members in non-native class + //'25', + //'26', + //'27', + // External instance members in native class + //'36', + //'37', + //'38', + // External static members in native class + //'39', + //'40', + //'41', + ]); }); } diff --git a/tests/compiler/dart2js_native/native_test.dart b/tests/compiler/dart2js_native/native_test.dart new file mode 100644 index 000000000000..0eb52d6b20a9 --- /dev/null +++ b/tests/compiler/dart2js_native/native_test.dart @@ -0,0 +1,211 @@ +// Copyright (c) 2018, 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. + +// Test for positive and negative uses of named declarations. This file is +// also used in tests/compiler/dart2js/model/native_test.dart. + +import 'dart:_js_helper'; + +var topLevelField; + +get topLevelGetter => null; + +set topLevelSetter(_) {} + +topLevelFunction() {} + +// NON_NATIVE_EXTERNAL //# 01: compile-time error +external get externalTopLevelGetter; //# 01: continued + +// NON_NATIVE_EXTERNAL //# 02: compile-time error +external set externalTopLevelSetter(_); //# 02: continued + +// NON_NATIVE_EXTERNAL //# 03: compile-time error +external externalTopLevelFunction(); //# 03: continued + +get nativeTopLevelGetter native; + +set nativeTopLevelSetter(_) native; + +nativeTopLevelFunction() native; + +class Class { + Class.generative(); + factory Class.fact() => null; + + // NON_NATIVE_EXTERNAL //# 08: compile-time error + external Class.externalGenerative(); //# 08: continued + + // NON_NATIVE_EXTERNAL //# 09: compile-time error + external factory Class.externalFact(); //# 09: continued + + // NATIVE_NON_INSTANCE_IN_NON_NATIVE_CLASS //# 10: compile-time error + Class.nativeGenerative() native; //# 10: continued + + // NATIVE_NON_INSTANCE_IN_NON_NATIVE_CLASS //# 11: compile-time error + factory Class.nativeFact() native; //# 11: continued + + var instanceField; + get instanceGetter => null; + set instanceSetter(_) {} + instanceMethod() {} + + static var staticField; + static get staticGetter => null; + static set staticSetter(_) {} + static staticMethod() {} + + // NON_NATIVE_EXTERNAL //# 22: compile-time error + external get externalInstanceGetter; //# 22: continued + + // NON_NATIVE_EXTERNAL //# 23: compile-time error + external set externalInstanceSetter(_); //# 23: continued + + // NON_NATIVE_EXTERNAL //# 24: compile-time error + external externalInstanceMethod(); //# 24: continued + + // NON_NATIVE_EXTERNAL //# 25: compile-time error + external static get externalStaticGetter; //# 25: continued + + // NON_NATIVE_EXTERNAL //# 26: compile-time error + external static set externalStaticSetter(_); //# 26: continued + + // NON_NATIVE_EXTERNAL //# 27: compile-time error + external static externalStaticMethod(); //# 27: continued + + get nativeInstanceGetter native; + set nativeInstanceSetter(_) native; + nativeInstanceMethod() native; + + // NATIVE_NON_INSTANCE_IN_NON_NATIVE_CLASS //# 28: compile-time error + static get nativeStaticGetter native; //# 28: continued + + // NATIVE_NON_INSTANCE_IN_NON_NATIVE_CLASS //# 29: compile-time error + static set nativeStaticSetter(_) native; //# 29: continued + + // NATIVE_NON_INSTANCE_IN_NON_NATIVE_CLASS //# 30: compile-time error + static nativeStaticMethod() native; //# 30: continued +} + +@Native('d') +class NativeClass { + NativeClass.generative(); + + factory NativeClass.fact() => null; + + // NON_NATIVE_EXTERNAL //# 31: compile-time error + external NativeClass.externalGenerative(); //# 31: continued + // NON_NATIVE_EXTERNAL //# 32: compile-time error + external factory NativeClass.externalFact(); //# 32: continued + + NativeClass.nativeGenerative() native; + factory NativeClass.nativeFact() native; + + var instanceField; + get instanceGetter => null; + set instanceSetter(_) {} + instanceMethod() {} + + static var staticField; + static get staticGetter => null; + static set staticSetter(_) {} + static staticMethod() {} + + var instanceNamedField; + + // NON_NATIVE_EXTERNAL //# 36: compile-time error + external get externalInstanceGetter; //# 36: continued + + // NON_NATIVE_EXTERNAL //# 37: compile-time error + external set externalInstanceSetter(_); //# 37: continued + + // NON_NATIVE_EXTERNAL //# 38: compile-time error + external externalInstanceMethod(); //# 38: continued + + // NON_NATIVE_EXTERNAL //# 39: compile-time error + external static get externalStaticGetter; //# 39: continued + + // NON_NATIVE_EXTERNAL //# 40: compile-time error + external static set externalStaticSetter(_); //# 40: continued + + // NON_NATIVE_EXTERNAL //# 41: compile-time error + external static externalStaticMethod(); //# 41: continued + + get nativeInstanceGetter native; + set nativeInstanceSetter(_) native; + nativeInstanceMethod() native; + + static get nativeStaticGetter native; + static set nativeStaticSetter(_) native; + static nativeStaticMethod() native; +} + +main() { + if (true) return; + + topLevelField; + topLevelGetter; + topLevelSetter = null; + topLevelFunction(); + externalTopLevelGetter; //# 01: continued + externalTopLevelSetter = null; //# 02: continued + externalTopLevelFunction(); //# 03: continued + nativeTopLevelGetter; + nativeTopLevelSetter = null; + nativeTopLevelFunction(); + + var c1 = new Class.generative(); + new Class.fact(); + new Class.externalGenerative(); //# 08: continued + new Class.externalFact(); //# 09: continued + new Class.nativeGenerative(); //# 10: continued + new Class.nativeFact(); //# 11: continued + c1.instanceField; + c1.instanceGetter; + c1.instanceSetter = null; + c1.instanceMethod(); + Class.staticField; + Class.staticGetter; + Class.staticSetter = null; + Class.staticMethod(); + c1.externalInstanceGetter; //# 22: continued + c1.externalInstanceSetter = null; //# 23: continued + c1.externalInstanceMethod(); //# 24: continued + Class.externalStaticGetter; //# 25: continued + Class.externalStaticSetter = null; //# 26: continued + Class.externalStaticMethod(); //# 27: continued + c1.nativeInstanceGetter; + c1.nativeInstanceSetter = null; + c1.nativeInstanceMethod(); + Class.nativeStaticGetter; //# 28: continued + Class.nativeStaticSetter = null; //# 29: continued + Class.nativeStaticMethod(); //# 30: continued + + var c2 = new NativeClass.generative(); + new NativeClass.fact(); + new NativeClass.externalGenerative(); //# 31: continued + new NativeClass.externalFact(); //# 32: continued + new NativeClass.nativeGenerative(); + new NativeClass.nativeFact(); + c2.instanceField; + c2.instanceGetter; + c2.instanceSetter = null; + c2.instanceMethod(); + NativeClass.staticField; + NativeClass.staticGetter; + NativeClass.staticSetter = null; + NativeClass.staticMethod(); + c2.externalInstanceGetter; //# 36: continued + c2.externalInstanceSetter = null; //# 37: continued + c2.externalInstanceMethod(); //# 38: continued + NativeClass.externalStaticGetter; //# 39: continued + NativeClass.externalStaticSetter = null; //# 40: continued + NativeClass.externalStaticMethod(); //# 41: continued + c2.nativeInstanceGetter; + c2.nativeInstanceSetter = null; + c2.nativeInstanceMethod(); + NativeClass.nativeStaticGetter; + NativeClass.nativeStaticSetter = null; + NativeClass.nativeStaticMethod(); +} diff --git a/third_party/googletest b/third_party/googletest new file mode 160000 index 000000000000..f854f1d27488 --- /dev/null +++ b/third_party/googletest @@ -0,0 +1 @@ +Subproject commit f854f1d27488996dc8a6db3c9453f80b02585e12