From 4cc88389fda650ea2be99440cf816582b36baa7b Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Thu, 3 Jun 2021 16:58:57 +0000 Subject: [PATCH 01/11] Issue 45672. Report constant errors when null value where non-nullable type is expected. When in a legacy library, runtimeTypeMatch() with the legacy type. Original: https://dart-review.googlesource.com/c/sdk/+/196042 Reverted as: https://dart-review.googlesource.com/c/sdk/+/196281 Presubmit looks green: https://test.corp.google.com/ui#id=OCL:369597863:BASE:377223944:1622729657782:14c50e7 Bug: https://github.com/dart-lang/sdk/issues/45672 Change-Id: Ife02dbdc4c111dec22536de4c50c349381ba1b3f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197109 Reviewed-by: Brian Wilkerson Commit-Queue: Konstantin Shcheglov --- .../lib/src/dart/constant/evaluation.dart | 7 +- .../test/generated/constant_test.dart | 2 +- ..._constructor_field_type_mismatch_test.dart | 94 ++++++++++++------- ...list_element_type_not_assignable_test.dart | 60 +++++++----- .../map_key_type_not_assignable_test.dart | 34 ++++++- .../map_value_type_not_assignable_test.dart | 34 ++++++- .../not_enough_positional_arguments_test.dart | 1 + .../set_element_type_not_assignable_test.dart | 89 ++++++++++++++---- .../constructor/bodyless_wrong_arg_test.dart | 2 + .../language/string/interpolation1_test.dart | 2 + 10 files changed, 247 insertions(+), 78 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/constant/evaluation.dart b/pkg/analyzer/lib/src/dart/constant/evaluation.dart index d8536a981b88a..68ce3c57ad1b0 100644 --- a/pkg/analyzer/lib/src/dart/constant/evaluation.dart +++ b/pkg/analyzer/lib/src/dart/constant/evaluation.dart @@ -822,11 +822,12 @@ class ConstantEvaluationEngine { DartObjectImpl obj, DartType type, ) { - if (obj.isNull) { - return true; + var typeSystem = library.typeSystem; + if (!typeSystem.isNonNullableByDefault) { + type = typeSystem.toLegacyType(type); } var objType = obj.type; - return library.typeSystem.isSubtypeOf(objType, type); + return typeSystem.isSubtypeOf(objType, type); } DartObjectImpl _nullObject(LibraryElementImpl library) { diff --git a/pkg/analyzer/test/generated/constant_test.dart b/pkg/analyzer/test/generated/constant_test.dart index 5b8847d418d3b..f4a40796acfe6 100644 --- a/pkg/analyzer/test/generated/constant_test.dart +++ b/pkg/analyzer/test/generated/constant_test.dart @@ -50,7 +50,7 @@ class C { "const Center(name: 'v')", context: ''' class Align { - final double widthFactor; + final double? widthFactor; const Align({String name, this.widthFactor}) assert(widthFactor == null || widthFactor >= 0.0); } diff --git a/pkg/analyzer/test/src/diagnostics/const_constructor_field_type_mismatch_test.dart b/pkg/analyzer/test/src/diagnostics/const_constructor_field_type_mismatch_test.dart index 1a3846a74dfb3..6f8fad03dbd0e 100644 --- a/pkg/analyzer/test/src/diagnostics/const_constructor_field_type_mismatch_test.dart +++ b/pkg/analyzer/test/src/diagnostics/const_constructor_field_type_mismatch_test.dart @@ -15,7 +15,7 @@ main() { @reflectiveTest class ConstConstructorFieldTypeMismatchTest extends PubPackageResolutionTest { - test_assignable_generic() async { + test_generic_int_int() async { await assertErrorsInCode( r''' class C { @@ -31,65 +31,95 @@ var v = const C(); ); } - test_assignable_nullValue() async { - await assertNoErrorsInCode(r''' -class A { - const A(x) : y = x; - final int y; + test_generic_string_int() async { + await assertErrorsInCode( + r''' +class C { + final T x = y; + const C(); } -var v = const A(null); -'''); +const int y = 1; +var v = const C(); +''', + [ + error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 27, 1), + error( + CompileTimeErrorCode.CONST_CONSTRUCTOR_FIELD_TYPE_MISMATCH, 70, 17), + ], + ); } - test_assignable_unresolvedFieldAndNullValue() async { + test_notGeneric_int_int() async { await assertErrorsInCode(r''' class A { const A(x) : y = x; - final Unresolved y; + final int y; } -var v = const A(null); +var v = const A('foo'); ''', [ - error(CompileTimeErrorCode.UNDEFINED_CLASS, 40, 10), + error(CompileTimeErrorCode.CONST_CONSTRUCTOR_FIELD_TYPE_MISMATCH, 57, 14), ]); } - test_notAssignable() async { + test_notGeneric_int_null() async { + var errors = expectedErrorsByNullability(nullable: [ + error(CompileTimeErrorCode.CONST_CONSTRUCTOR_FIELD_TYPE_MISMATCH, 57, 13), + ], legacy: []); await assertErrorsInCode(r''' class A { const A(x) : y = x; final int y; } -var v = const A('foo'); +var v = const A(null); +''', errors); + } + + test_notGeneric_null_forNonNullable_fromLegacy() async { + newFile('$testPackageLibPath/a.dart', content: r''' +class C { + final int f; + const C(a) : f = a; +} +'''); + await assertNoErrorsInCode(''' +// @dart = 2.9 +import 'a.dart'; +const a = const C(null); +'''); + } + + test_notGeneric_null_forNonNullable_fromNullSafe() async { + await assertErrorsInCode(''' +class C { + final int f; + const C(a) : f = a; +} + +const a = const C(null); ''', [ - error(CompileTimeErrorCode.CONST_CONSTRUCTOR_FIELD_TYPE_MISMATCH, 57, 14), + error(CompileTimeErrorCode.CONST_CONSTRUCTOR_FIELD_TYPE_MISMATCH, 60, 13), ]); } - test_notAssignable_generic() async { - await assertErrorsInCode( - r''' -class C { - final T x = y; - const C(); + test_notGeneric_unresolved_int() async { + await assertErrorsInCode(r''' +class A { + const A(x) : y = x; + final Unresolved y; } -const int y = 1; -var v = const C(); -''', - [ - error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 27, 1), - error( - CompileTimeErrorCode.CONST_CONSTRUCTOR_FIELD_TYPE_MISMATCH, 70, 17), - ], - ); +var v = const A(0); +''', [ + error(CompileTimeErrorCode.UNDEFINED_CLASS, 40, 10), + ]); } - test_notAssignable_unresolved() async { + test_notGeneric_unresolved_null() async { await assertErrorsInCode(r''' class A { const A(x) : y = x; final Unresolved y; } -var v = const A('foo'); +var v = const A(null); ''', [ error(CompileTimeErrorCode.UNDEFINED_CLASS, 40, 10), ]); diff --git a/pkg/analyzer/test/src/diagnostics/list_element_type_not_assignable_test.dart b/pkg/analyzer/test/src/diagnostics/list_element_type_not_assignable_test.dart index f15fcae43f51c..76204efa364ef 100644 --- a/pkg/analyzer/test/src/diagnostics/list_element_type_not_assignable_test.dart +++ b/pkg/analyzer/test/src/diagnostics/list_element_type_not_assignable_test.dart @@ -11,12 +11,19 @@ main() { defineReflectiveSuite(() { defineReflectiveTests(ListElementTypeNotAssignableTest); defineReflectiveTests(ListElementTypeNotAssignableWithNoImplicitCastsTest); + defineReflectiveTests(ListElementTypeNotAssignableWithoutNullSafetyTest); }); } @reflectiveTest class ListElementTypeNotAssignableTest extends PubPackageResolutionTest - with ListElementTypeNotAssignableTestCases {} + with ListElementTypeNotAssignableTestCases { + test_const_stringQuestion_null_value() async { + await assertNoErrorsInCode(''' +var v = const [null]; +'''); + } +} mixin ListElementTypeNotAssignableTestCases on PubPackageResolutionTest { test_const_ifElement_thenElseFalse_intInt() async { @@ -68,6 +75,32 @@ var v = const [if (true) a]; ]); } + test_const_intInt() async { + await assertNoErrorsInCode(r''' +var v1 = [42]; +var v2 = const [42]; +'''); + } + + test_const_intNull_dynamic() async { + var errors = expectedErrorsByNullability(nullable: [ + error(CompileTimeErrorCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE, 36, 1), + ], legacy: []); + await assertErrorsInCode(''' +const a = null; +var v = const [a]; +''', errors); + } + + test_const_intNull_value() async { + var errors = expectedErrorsByNullability(nullable: [ + error(CompileTimeErrorCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE, 20, 4), + ], legacy: []); + await assertErrorsInCode(''' +var v = const [null]; +''', errors); + } + test_const_spread_intInt() async { await assertNoErrorsInCode(''' var v = const [...[0, 1]]; @@ -91,32 +124,12 @@ var v = const [x]; ]); } - test_const_stringNull() async { - await assertNoErrorsInCode(''' -var v = const [null]; -'''); - } - - test_const_stringNull_dynamic() async { - await assertNoErrorsInCode(''' -const dynamic x = null; -var v = const [x]; -'''); - } - test_const_voidInt() async { await assertNoErrorsInCode(''' var v = const [42]; '''); } - test_element_type_is_assignable() async { - await assertNoErrorsInCode(r''' -var v1 = [42]; -var v2 = const [42]; -'''); - } - test_nonConst_ifElement_thenElseFalse_intDynamic() async { await assertNoErrorsInCode(''' const dynamic a = 'a'; @@ -237,3 +250,8 @@ void f(Iterable a) { ]); } } + +@reflectiveTest +class ListElementTypeNotAssignableWithoutNullSafetyTest + extends PubPackageResolutionTest + with WithoutNullSafetyMixin, ListElementTypeNotAssignableTestCases {} diff --git a/pkg/analyzer/test/src/diagnostics/map_key_type_not_assignable_test.dart b/pkg/analyzer/test/src/diagnostics/map_key_type_not_assignable_test.dart index c1b9c6dab3a73..1216de79d930c 100644 --- a/pkg/analyzer/test/src/diagnostics/map_key_type_not_assignable_test.dart +++ b/pkg/analyzer/test/src/diagnostics/map_key_type_not_assignable_test.dart @@ -17,7 +17,20 @@ main() { @reflectiveTest class MapKeyTypeNotAssignableTest extends PubPackageResolutionTest - with MapKeyTypeNotAssignableTestCases {} + with MapKeyTypeNotAssignableTestCases { + test_const_intQuestion_null_dynamic() async { + await assertNoErrorsInCode(''' +const dynamic a = null; +var v = const {a : true}; +'''); + } + + test_const_intQuestion_null_value() async { + await assertNoErrorsInCode(''' +var v = const {null : true}; +'''); + } +} mixin MapKeyTypeNotAssignableTestCases on PubPackageResolutionTest { test_const_ifElement_thenElseFalse_intInt_dynamic() async { @@ -85,6 +98,25 @@ var v = const {a : true}; '''); } + test_const_intNull_dynamic() async { + var errors = expectedErrorsByNullability(nullable: [ + error(CompileTimeErrorCode.MAP_KEY_TYPE_NOT_ASSIGNABLE, 50, 1), + ], legacy: []); + await assertErrorsInCode(''' +const dynamic a = null; +var v = const {a : true}; +''', errors); + } + + test_const_intNull_value() async { + var errors = expectedErrorsByNullability(nullable: [ + error(CompileTimeErrorCode.MAP_KEY_TYPE_NOT_ASSIGNABLE, 26, 4), + ], legacy: []); + await assertErrorsInCode(''' +var v = const {null : true}; +''', errors); + } + test_const_intString_dynamic() async { await assertErrorsInCode(''' const dynamic a = 'a'; diff --git a/pkg/analyzer/test/src/diagnostics/map_value_type_not_assignable_test.dart b/pkg/analyzer/test/src/diagnostics/map_value_type_not_assignable_test.dart index cbcfd6c824487..212b70b36633f 100644 --- a/pkg/analyzer/test/src/diagnostics/map_value_type_not_assignable_test.dart +++ b/pkg/analyzer/test/src/diagnostics/map_value_type_not_assignable_test.dart @@ -17,7 +17,20 @@ main() { @reflectiveTest class MapValueTypeNotAssignableTest extends PubPackageResolutionTest - with MapValueTypeNotAssignableTestCases {} + with MapValueTypeNotAssignableTestCases { + test_const_intQuestion_null_dynamic() async { + await assertNoErrorsInCode(''' +const dynamic a = null; +var v = const {true: a}; +'''); + } + + test_const_intQuestion_null_value() async { + await assertNoErrorsInCode(''' +var v = const {true: null}; +'''); + } +} mixin MapValueTypeNotAssignableTestCases on PubPackageResolutionTest { test_const_ifElement_thenElseFalse_intInt_dynamic() async { @@ -85,6 +98,25 @@ var v = const {true: a}; '''); } + test_const_intNull_dynamic() async { + var errors = expectedErrorsByNullability(nullable: [ + error(CompileTimeErrorCode.MAP_VALUE_TYPE_NOT_ASSIGNABLE, 56, 1), + ], legacy: []); + await assertErrorsInCode(''' +const dynamic a = null; +var v = const {true: a}; +''', errors); + } + + test_const_intNull_value() async { + var errors = expectedErrorsByNullability(nullable: [ + error(CompileTimeErrorCode.MAP_VALUE_TYPE_NOT_ASSIGNABLE, 32, 4), + ], legacy: []); + await assertErrorsInCode(''' +var v = const {true: null}; +''', errors); + } + test_const_intString_dynamic() async { await assertErrorsInCode(''' const dynamic a = 'a'; diff --git a/pkg/analyzer/test/src/diagnostics/not_enough_positional_arguments_test.dart b/pkg/analyzer/test/src/diagnostics/not_enough_positional_arguments_test.dart index b08bd582accfc..c3ca2c0433f69 100644 --- a/pkg/analyzer/test/src/diagnostics/not_enough_positional_arguments_test.dart +++ b/pkg/analyzer/test/src/diagnostics/not_enough_positional_arguments_test.dart @@ -24,6 +24,7 @@ main() { const A(); } ''', [ + error(CompileTimeErrorCode.CONST_CONSTRUCTOR_PARAM_TYPE_MISMATCH, 41, 9), error(CompileTimeErrorCode.NOT_ENOUGH_POSITIONAL_ARGUMENTS, 48, 2), ]); } diff --git a/pkg/analyzer/test/src/diagnostics/set_element_type_not_assignable_test.dart b/pkg/analyzer/test/src/diagnostics/set_element_type_not_assignable_test.dart index 88efa36542dd7..a74f2ccf9b04a 100644 --- a/pkg/analyzer/test/src/diagnostics/set_element_type_not_assignable_test.dart +++ b/pkg/analyzer/test/src/diagnostics/set_element_type_not_assignable_test.dart @@ -11,12 +11,26 @@ main() { defineReflectiveSuite(() { defineReflectiveTests(SetElementTypeNotAssignableTest); defineReflectiveTests(SetElementTypeNotAssignableWithNoImplicitCastsTest); + defineReflectiveTests(SetElementTypeNotAssignableWithoutNullSafetyTest); }); } @reflectiveTest class SetElementTypeNotAssignableTest extends PubPackageResolutionTest - with SetElementTypeNotAssignableTestCases {} + with SetElementTypeNotAssignableTestCases { + test_const_stringQuestion_null_dynamic() async { + await assertNoErrorsInCode(''' +const a = null; +var v = const {a}; +'''); + } + + test_const_stringQuestion_null_value() async { + await assertNoErrorsInCode(''' +var v = const {null}; +'''); + } +} mixin SetElementTypeNotAssignableTestCases on PubPackageResolutionTest { test_const_ifElement_thenElseFalse_intInt() async { @@ -68,44 +82,61 @@ var v = const {if (true) a}; ]); } - test_const_spread_intInt() async { + test_const_intInt_dynamic() async { await assertNoErrorsInCode(''' -var v = const {...[0, 1]}; +const dynamic a = 42; +var v = const {a}; '''); } - test_explicitTypeArgs_const() async { + test_const_intInt_value() async { + await assertNoErrorsInCode(''' +var v = const {42}; +'''); + } + + test_const_intNull_dynamic() async { + var errors = expectedErrorsByNullability(nullable: [ + error(CompileTimeErrorCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE, 36, 1), + ], legacy: []); await assertErrorsInCode(''' -var v = const {42}; -''', [ - error(CompileTimeErrorCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE, 23, 2), - ]); +const a = null; +var v = const {a}; +''', errors); } - test_explicitTypeArgs_const_actualTypeMatch() async { - await assertNoErrorsInCode(''' -const dynamic x = null; -var v = const {x}; -'''); + test_const_intNull_value() async { + var errors = expectedErrorsByNullability(nullable: [ + error(CompileTimeErrorCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE, 20, 4), + ], legacy: []); + await assertErrorsInCode(''' +var v = const {null}; +''', errors); } - test_explicitTypeArgs_const_actualTypeMismatch() async { + test_const_intString_dynamic() async { await assertErrorsInCode(''' -const dynamic x = 42; -var v = const {x}; +const dynamic x = 'abc'; +var v = const {x}; ''', [ error(CompileTimeErrorCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE, 45, 1), ]); } - test_explicitTypeArgs_notConst() async { + test_const_intString_value() async { await assertErrorsInCode(''' -var v = {42}; +var v = const {'abc'}; ''', [ - error(CompileTimeErrorCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE, 17, 2), + error(CompileTimeErrorCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE, 20, 5), ]); } + test_const_spread_intInt() async { + await assertNoErrorsInCode(''' +var v = const {...[0, 1]}; +'''); + } + test_nonConst_ifElement_thenElseFalse_intDynamic() async { await assertNoErrorsInCode(''' const dynamic a = 'a'; @@ -149,6 +180,21 @@ var v = {if (true) a}; var v = {...[0, 1]}; '''); } + + test_notConst_intString_dynamic() async { + await assertNoErrorsInCode(''' +const dynamic x = 'abc'; +var v = {x}; +'''); + } + + test_notConst_intString_value() async { + await assertErrorsInCode(''' +var v = {'abc'}; +''', [ + error(CompileTimeErrorCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE, 14, 5), + ]); + } } @reflectiveTest @@ -205,3 +251,8 @@ void f(Iterable a) { ]); } } + +@reflectiveTest +class SetElementTypeNotAssignableWithoutNullSafetyTest + extends PubPackageResolutionTest + with WithoutNullSafetyMixin, SetElementTypeNotAssignableTestCases {} diff --git a/tests/language/constructor/bodyless_wrong_arg_test.dart b/tests/language/constructor/bodyless_wrong_arg_test.dart index 4e669d03edde1..51f16f56c18bf 100644 --- a/tests/language/constructor/bodyless_wrong_arg_test.dart +++ b/tests/language/constructor/bodyless_wrong_arg_test.dart @@ -18,4 +18,6 @@ class C extends Base { main() { const C("str"); +//^^^^^^^^^^^^^^ +// [analyzer] COMPILE_TIME_ERROR.CONST_EVAL_THROWS_EXCEPTION } diff --git a/tests/language/string/interpolation1_test.dart b/tests/language/string/interpolation1_test.dart index 7f43236fb6947..d5eaea980cd66 100644 --- a/tests/language/string/interpolation1_test.dart +++ b/tests/language/string/interpolation1_test.dart @@ -12,6 +12,8 @@ class A { class StringInterpolation1NegativeTest { // Dollar not followed by "{" or identifier. static const DOLLAR = const A("$"); + // [error line 14, column 33, length 3] + // [analyzer] COMPILE_TIME_ERROR.CONST_CONSTRUCTOR_PARAM_TYPE_MISMATCH // [error line 14, column 35, length 0] // [analyzer] COMPILE_TIME_ERROR.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE // [cfe] A '$' has special meaning inside a string, and must be followed by an identifier or an expression in curly braces ({}). From 6589ca13bce4460474ad7c4b500aecce7b95bb38 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Thu, 3 Jun 2021 17:05:38 +0000 Subject: [PATCH 02/11] Revert "[vm] Remove Script::compile_time_constants_." This reverts commit a10e038f092b1177748674eaf83c52b3cbf882c3. Reason for revert: Android ARM32 disagrees on how UntaggedScript is packed Original change's description: > [vm] Remove Script::compile_time_constants_. > > Dead since 5cce1e4acde9c166877a0c1a142e0cb458964478. > > TEST=ci > Change-Id: If55de08b753e5948785187e455ac793356d1e794 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202066 > Commit-Queue: Ryan Macnak > Reviewed-by: Ben Konyi TBR=bkonyi@google.com,rmacnak@google.com Change-Id: I41872aa99af056d4a2c730403628a86b61185e46 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202300 Reviewed-by: Ryan Macnak Commit-Queue: Ryan Macnak --- .../vm/compiler/runtime_offsets_extracted.h | 24 +++++++++---------- runtime/vm/object.cc | 4 ++++ runtime/vm/object.h | 5 ++++ runtime/vm/raw_object.h | 5 ++-- runtime/vm/raw_object_fields.cc | 1 + 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/runtime/vm/compiler/runtime_offsets_extracted.h b/runtime/vm/compiler/runtime_offsets_extracted.h index 56e8f0e52953e..a88073426b2e9 100644 --- a/runtime/vm/compiler/runtime_offsets_extracted.h +++ b/runtime/vm/compiler/runtime_offsets_extracted.h @@ -527,7 +527,7 @@ static constexpr dart::compiler::target::word PcDescriptors_HeaderSize = 8; static constexpr dart::compiler::target::word Pointer_InstanceSize = 12; static constexpr dart::compiler::target::word ReceivePort_InstanceSize = 20; static constexpr dart::compiler::target::word RegExp_InstanceSize = 60; -static constexpr dart::compiler::target::word Script_InstanceSize = 48; +static constexpr dart::compiler::target::word Script_InstanceSize = 56; static constexpr dart::compiler::target::word SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word SingleTargetCache_InstanceSize = 16; @@ -1072,7 +1072,7 @@ static constexpr dart::compiler::target::word PcDescriptors_HeaderSize = 16; static constexpr dart::compiler::target::word Pointer_InstanceSize = 24; static constexpr dart::compiler::target::word ReceivePort_InstanceSize = 40; static constexpr dart::compiler::target::word RegExp_InstanceSize = 120; -static constexpr dart::compiler::target::word Script_InstanceSize = 80; +static constexpr dart::compiler::target::word Script_InstanceSize = 96; static constexpr dart::compiler::target::word SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word SingleTargetCache_InstanceSize = 32; @@ -1607,7 +1607,7 @@ static constexpr dart::compiler::target::word PcDescriptors_HeaderSize = 8; static constexpr dart::compiler::target::word Pointer_InstanceSize = 12; static constexpr dart::compiler::target::word ReceivePort_InstanceSize = 20; static constexpr dart::compiler::target::word RegExp_InstanceSize = 60; -static constexpr dart::compiler::target::word Script_InstanceSize = 48; +static constexpr dart::compiler::target::word Script_InstanceSize = 56; static constexpr dart::compiler::target::word SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word SingleTargetCache_InstanceSize = 16; @@ -2153,7 +2153,7 @@ static constexpr dart::compiler::target::word PcDescriptors_HeaderSize = 16; static constexpr dart::compiler::target::word Pointer_InstanceSize = 24; static constexpr dart::compiler::target::word ReceivePort_InstanceSize = 40; static constexpr dart::compiler::target::word RegExp_InstanceSize = 120; -static constexpr dart::compiler::target::word Script_InstanceSize = 80; +static constexpr dart::compiler::target::word Script_InstanceSize = 96; static constexpr dart::compiler::target::word SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word SingleTargetCache_InstanceSize = 32; @@ -2697,7 +2697,7 @@ static constexpr dart::compiler::target::word PcDescriptors_HeaderSize = 16; static constexpr dart::compiler::target::word Pointer_InstanceSize = 24; static constexpr dart::compiler::target::word ReceivePort_InstanceSize = 24; static constexpr dart::compiler::target::word RegExp_InstanceSize = 80; -static constexpr dart::compiler::target::word Script_InstanceSize = 56; +static constexpr dart::compiler::target::word Script_InstanceSize = 64; static constexpr dart::compiler::target::word SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word SingleTargetCache_InstanceSize = 32; @@ -3242,7 +3242,7 @@ static constexpr dart::compiler::target::word PcDescriptors_HeaderSize = 16; static constexpr dart::compiler::target::word Pointer_InstanceSize = 24; static constexpr dart::compiler::target::word ReceivePort_InstanceSize = 24; static constexpr dart::compiler::target::word RegExp_InstanceSize = 80; -static constexpr dart::compiler::target::word Script_InstanceSize = 56; +static constexpr dart::compiler::target::word Script_InstanceSize = 64; static constexpr dart::compiler::target::word SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word SingleTargetCache_InstanceSize = 32; @@ -7062,7 +7062,7 @@ static constexpr dart::compiler::target::word AOT_PcDescriptors_HeaderSize = 8; static constexpr dart::compiler::target::word AOT_Pointer_InstanceSize = 12; static constexpr dart::compiler::target::word AOT_ReceivePort_InstanceSize = 20; static constexpr dart::compiler::target::word AOT_RegExp_InstanceSize = 60; -static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 40; +static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 48; static constexpr dart::compiler::target::word AOT_SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_SingleTargetCache_InstanceSize = 16; @@ -7669,7 +7669,7 @@ static constexpr dart::compiler::target::word AOT_PcDescriptors_HeaderSize = 16; static constexpr dart::compiler::target::word AOT_Pointer_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_ReceivePort_InstanceSize = 40; static constexpr dart::compiler::target::word AOT_RegExp_InstanceSize = 120; -static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 72; +static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 80; static constexpr dart::compiler::target::word AOT_SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_SingleTargetCache_InstanceSize = 32; @@ -8280,7 +8280,7 @@ static constexpr dart::compiler::target::word AOT_PcDescriptors_HeaderSize = 16; static constexpr dart::compiler::target::word AOT_Pointer_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_ReceivePort_InstanceSize = 40; static constexpr dart::compiler::target::word AOT_RegExp_InstanceSize = 120; -static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 72; +static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 80; static constexpr dart::compiler::target::word AOT_SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_SingleTargetCache_InstanceSize = 32; @@ -10093,7 +10093,7 @@ static constexpr dart::compiler::target::word AOT_PcDescriptors_HeaderSize = 8; static constexpr dart::compiler::target::word AOT_Pointer_InstanceSize = 12; static constexpr dart::compiler::target::word AOT_ReceivePort_InstanceSize = 12; static constexpr dart::compiler::target::word AOT_RegExp_InstanceSize = 60; -static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 40; +static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 48; static constexpr dart::compiler::target::word AOT_SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_SingleTargetCache_InstanceSize = 16; @@ -10693,7 +10693,7 @@ static constexpr dart::compiler::target::word AOT_PcDescriptors_HeaderSize = 16; static constexpr dart::compiler::target::word AOT_Pointer_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_ReceivePort_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_RegExp_InstanceSize = 120; -static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 72; +static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 80; static constexpr dart::compiler::target::word AOT_SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_SingleTargetCache_InstanceSize = 32; @@ -11297,7 +11297,7 @@ static constexpr dart::compiler::target::word AOT_PcDescriptors_HeaderSize = 16; static constexpr dart::compiler::target::word AOT_Pointer_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_ReceivePort_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_RegExp_InstanceSize = 120; -static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 72; +static constexpr dart::compiler::target::word AOT_Script_InstanceSize = 80; static constexpr dart::compiler::target::word AOT_SendPort_InstanceSize = 24; static constexpr dart::compiler::target::word AOT_SingleTargetCache_InstanceSize = 32; diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index cf4a20dce2cab..571b3901ea146 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -11603,6 +11603,10 @@ void Script::LoadSourceFromKernel(const uint8_t* kernel_buffer, } #endif // !defined(DART_PRECOMPILED_RUNTIME) +void Script::set_compile_time_constants(const Array& value) const { + untag()->set_compile_time_constants(value.ptr()); +} + void Script::set_kernel_program_info(const KernelProgramInfo& info) const { untag()->set_kernel_program_info(info.ptr()); } diff --git a/runtime/vm/object.h b/runtime/vm/object.h index fe2153979ae2f..065e42dd65650 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -4464,6 +4464,11 @@ class Script : public Object { // The load time in milliseconds since epoch. int64_t load_timestamp() const { return untag()->load_timestamp_; } + ArrayPtr compile_time_constants() const { + return untag()->compile_time_constants(); + } + void set_compile_time_constants(const Array& value) const; + KernelProgramInfoPtr kernel_program_info() const { return untag()->kernel_program_info(); } diff --git a/runtime/vm/raw_object.h b/runtime/vm/raw_object.h index 8d3185d93f912..da4c33a801a12 100644 --- a/runtime/vm/raw_object.h +++ b/runtime/vm/raw_object.h @@ -1567,6 +1567,7 @@ class alignas(8) UntaggedScript : public UntaggedObject { COMPRESSED_POINTER_FIELD(StringPtr, url) VISIT_FROM(url) COMPRESSED_POINTER_FIELD(StringPtr, resolved_url) + COMPRESSED_POINTER_FIELD(ArrayPtr, compile_time_constants) COMPRESSED_POINTER_FIELD(TypedDataPtr, line_starts) #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME) COMPRESSED_POINTER_FIELD(ExternalTypedDataPtr, constant_coverage) @@ -1615,11 +1616,11 @@ class alignas(8) UntaggedScript : public UntaggedObject { #endif #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME) - int32_t kernel_script_index_; int64_t load_timestamp_; + int32_t kernel_script_index_; #else - int64_t load_timestamp_; int32_t kernel_script_index_; + int64_t load_timestamp_; #endif }; diff --git a/runtime/vm/raw_object_fields.cc b/runtime/vm/raw_object_fields.cc index 1b09c409c88fb..629d62b3f03a7 100644 --- a/runtime/vm/raw_object_fields.cc +++ b/runtime/vm/raw_object_fields.cc @@ -43,6 +43,7 @@ namespace dart { F(Field, host_offset_or_field_id_) \ F(Script, url_) \ F(Script, resolved_url_) \ + F(Script, compile_time_constants_) \ F(Script, line_starts_) \ F(Script, debug_positions_) \ F(Script, kernel_program_info_) \ From 2383d4c579037f0aebef21ff91a8f5b63be96289 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Thu, 3 Jun 2021 17:17:14 +0000 Subject: [PATCH 03/11] pkg:wasm: Update lints to pkg:lints, cleanup setup.dart - Standardize on `dart run wasm:setup` - Cleanup in readme Change-Id: I98099cead7be715aa804397e73aac74197713f69 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202145 Auto-Submit: Kevin Moore Reviewed-by: Liam Appelbe Commit-Queue: Liam Appelbe --- pkg/wasm/README.md | 10 ++-- pkg/wasm/analysis_options.yaml | 5 +- pkg/wasm/bin/setup.dart | 83 ++++++++++++++++++---------------- pkg/wasm/pubspec.yaml | 2 +- 4 files changed, 55 insertions(+), 45 deletions(-) diff --git a/pkg/wasm/README.md b/pkg/wasm/README.md index ecf418df29fcd..4558a3c179f0f 100644 --- a/pkg/wasm/README.md +++ b/pkg/wasm/README.md @@ -4,7 +4,7 @@ Built on top of the [Wasmer](https://github.com/wasmerio/wasmer) runtime. ## Setup -Run `dart bin/setup.dart` to build the Wasmer runtime. +Run `dart run wasm:setup` to build the Wasmer runtime. ## Basic Usage @@ -25,15 +25,15 @@ clang --target=wasm32 -nostdlib -Wl,--export-all -Wl,--no-entry -o square.wasm s Then we can load and run it like this: ```dart -import "dart:io"; -import "package:wasm/wasm.dart"; +import 'dart:io'; +import 'package:wasm/wasm.dart'; void main() { - final data = File("square.wasm").readAsBytesSync(); + final data = File('square.wasm').readAsBytesSync(); final mod = WasmModule(data); print(mod.describe()); final inst = mod.instantiate().build(); - final square = inst.lookupFunction("square"); + final square = inst.lookupFunction('square'); print(square(12)); } ``` diff --git a/pkg/wasm/analysis_options.yaml b/pkg/wasm/analysis_options.yaml index 2bb8d1dccd601..74fd4c87ce2ba 100644 --- a/pkg/wasm/analysis_options.yaml +++ b/pkg/wasm/analysis_options.yaml @@ -1,4 +1,4 @@ -include: package:pedantic/analysis_options.1.9.0.yaml +include: package:lints/recommended.yaml analyzer: strong-mode: @@ -15,6 +15,7 @@ linter: - avoid_returning_null_for_void - avoid_unused_constructor_parameters - await_only_futures + - cancel_subscriptions - camel_case_types - cascade_invocations - constant_identifier_names @@ -44,6 +45,7 @@ linter: - prefer_interpolation_to_compose_strings - prefer_is_not_operator - prefer_null_aware_operators + - prefer_single_quotes - prefer_relative_imports - prefer_typing_uninitialized_variables - prefer_void_to_null @@ -53,6 +55,7 @@ linter: - test_types_in_equals - throw_in_finally - type_annotate_public_apis + - unawaited_futures - unnecessary_brace_in_string_interps - unnecessary_lambdas - unnecessary_null_aware_assignments diff --git a/pkg/wasm/bin/setup.dart b/pkg/wasm/bin/setup.dart index 04fc145f47a95..ac713b5dcd112 100644 --- a/pkg/wasm/bin/setup.dart +++ b/pkg/wasm/bin/setup.dart @@ -5,12 +5,31 @@ // Builds the wasmer runtime library, to by used by package:wasm. Requires // rustc, cargo, clang, and clang++. If a target triple is not specified, it // will default to the host target. -// Usage: dart setup.dart [target-triple] +// Usage: dart run wasm:setup [target-triple] import 'dart:convert'; -import 'dart:io'; +import 'dart:io' hide exit; -Uri getSdkDir() { +Future main(List args) async { + if (args.length > 1) { + print('Usage: dart run wasm:setup [target-triple]'); + exitCode = 64; // bad usage + return; + } + + final target = args.isNotEmpty ? args[0] : await _getTargetTriple(); + + try { + await _main(target); + } on ProcessException catch (e) { + final invocation = [e.executable, ...e.arguments].join(' '); + print('FAILED with exit code ${e.errorCode} `$invocation`'); + exitCode = 70; // software error + return; + } +} + +Uri _getSdkDir() { // The common case, and how cli_util.dart computes the Dart SDK directory, // path.dirname called twice on Platform.resolvedExecutable. final exe = Uri.file(Platform.resolvedExecutable); @@ -37,7 +56,7 @@ Uri getSdkDir() { return commonSdkDir; } -Uri getOutDir(Uri root) { +Uri _getOutDir(Uri root) { // Traverse up until we see a `.dart_tool/package_config.json` file. do { if (File.fromUri(root.resolve('.dart_tool/package_config.json')) @@ -48,7 +67,7 @@ Uri getOutDir(Uri root) { throw Exception('.dart_tool/package_config.json not found'); } -String getOutLib(String target) { +String _getOutLib(String target) { final os = RegExp(r'^.*-.*-(.*)').firstMatch(target)?.group(1) ?? ''; if (os == 'darwin' || os == 'ios') { return 'libwasmer.dylib'; @@ -58,9 +77,10 @@ String getOutLib(String target) { return 'libwasmer.so'; } -Future getTargetTriple() async { +Future _getTargetTriple() async { + final _regexp = RegExp(r'^([^=]+)="(.*)"$'); final process = await Process.start('rustc', ['--print', 'cfg']); - process.stderr + final sub = process.stderr .transform(utf8.decoder) .transform(const LineSplitter()) .listen((line) => stderr.writeln(line)); @@ -68,10 +88,11 @@ Future getTargetTriple() async { await process.stdout .transform(utf8.decoder) .transform(const LineSplitter()) - .listen((line) { - final match = RegExp(r'^([^=]+)="(.*)"$').firstMatch(line); + .forEach((line) { + final match = _regexp.firstMatch(line); if (match != null) cfg[match.group(1)!] = match.group(2); - }).asFuture(); + }); + await sub.cancel(); var arch = cfg['target_arch'] ?? 'unknown'; var vendor = cfg['target_vendor'] ?? 'unknown'; var os = cfg['target_os'] ?? 'unknown'; @@ -82,35 +103,21 @@ Future getTargetTriple() async { .join('-'); } -Future run(String exe, List args) async { +Future _run(String exe, List args) async { print('\n$exe ${args.join(' ')}\n'); - final process = await Process.start(exe, args); - process.stdout - .transform(utf8.decoder) - .transform(const LineSplitter()) - .listen(print); - process.stderr - .transform(utf8.decoder) - .transform(const LineSplitter()) - .listen((line) => stderr.writeln(line)); - final exitCode = await process.exitCode; - if (exitCode != 0) { - print('Command failed with exit code $exitCode'); - exit(exitCode); + final process = + await Process.start(exe, args, mode: ProcessStartMode.inheritStdio); + final result = await process.exitCode; + if (result != 0) { + throw ProcessException(exe, args, '', result); } } -Future main(List args) async { - if (args.length > 1) { - print('Usage: dart setup.dart [target-triple]'); - exit(1); - } - - final target = args.isNotEmpty ? args[0] : await getTargetTriple(); - final sdkDir = getSdkDir(); +Future _main(String target) async { + final sdkDir = _getSdkDir(); final binDir = Platform.script; - final outDir = getOutDir(binDir); - final outLib = outDir.resolve(getOutLib(target)).path; + final outDir = _getOutDir(binDir); + final outLib = outDir.resolve(_getOutLib(target)).path; print('Dart SDK directory: ${sdkDir.path}'); print('Script directory: ${binDir.path}'); @@ -119,7 +126,7 @@ Future main(List args) async { print('Output library: $outLib'); // Build wasmer crate. - await run('cargo', [ + await _run('cargo', [ 'build', '--target', target, @@ -141,7 +148,7 @@ Future main(List args) async { } // Build dart_api_dl.o. - await run('clang', [ + await _run('clang', [ '-DDART_SHARED_LIB', '-DNDEBUG', '-fno-exceptions', @@ -160,7 +167,7 @@ Future main(List args) async { ]); // Build finalizers.o. - await run('clang++', [ + await _run('clang++', [ '-DDART_SHARED_LIB', '-DNDEBUG', '-fno-exceptions', @@ -181,7 +188,7 @@ Future main(List args) async { ]); // Link wasmer, dart_api_dl, and finalizers to create the output library. - await run('clang++', [ + await _run('clang++', [ '-shared', '-fPIC', '-target', diff --git a/pkg/wasm/pubspec.yaml b/pkg/wasm/pubspec.yaml index 9cd60c7e66a06..a91c321c12e20 100644 --- a/pkg/wasm/pubspec.yaml +++ b/pkg/wasm/pubspec.yaml @@ -10,5 +10,5 @@ dependencies: ffi: ^1.0.0 dev_dependencies: - pedantic: ^1.10.0 + lints: ^1.0.0 test: ^1.16.0 From 9cf93054eee2e93c0a86a8c3ed9f7c7c0c1f5c06 Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Thu, 3 Jun 2021 17:26:07 +0000 Subject: [PATCH 04/11] [ benchmark ] Fix names for SDKArtifactSizes benchmark outputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I569723b04136d37607a61e5a47a2e8661425f820 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202263 Reviewed-by: Régis Crelier Commit-Queue: Ben Konyi --- benchmarks/SDKArtifactSizes/dart/SDKArtifactSizes.dart | 2 +- benchmarks/SDKArtifactSizes/dart2/SDKArtifactSizes.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/benchmarks/SDKArtifactSizes/dart/SDKArtifactSizes.dart b/benchmarks/SDKArtifactSizes/dart/SDKArtifactSizes.dart index d914254cd5adc..1130ac6926a44 100644 --- a/benchmarks/SDKArtifactSizes/dart/SDKArtifactSizes.dart +++ b/benchmarks/SDKArtifactSizes/dart/SDKArtifactSizes.dart @@ -35,7 +35,7 @@ const snapshots = [ Future reportArtifactSize(String path, String name) async { final size = await File(path).length(); - print('SDKArtifactSize.$name(CodeSize): $size'); + print('SDKArtifactSizes.$name(CodeSize): $size'); } Future main() async { diff --git a/benchmarks/SDKArtifactSizes/dart2/SDKArtifactSizes.dart b/benchmarks/SDKArtifactSizes/dart2/SDKArtifactSizes.dart index d914254cd5adc..1130ac6926a44 100644 --- a/benchmarks/SDKArtifactSizes/dart2/SDKArtifactSizes.dart +++ b/benchmarks/SDKArtifactSizes/dart2/SDKArtifactSizes.dart @@ -35,7 +35,7 @@ const snapshots = [ Future reportArtifactSize(String path, String name) async { final size = await File(path).length(); - print('SDKArtifactSize.$name(CodeSize): $size'); + print('SDKArtifactSizes.$name(CodeSize): $size'); } Future main() async { From 4b5c275d2caaca0bae5729b0a107551f664a9741 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Thu, 3 Jun 2021 17:39:47 +0000 Subject: [PATCH 05/11] Sort api_test.dart Change-Id: I1beb4c553b0e9b03ce9c37d3507ec33ed44269ea Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202260 Reviewed-by: Konstantin Shcheglov Reviewed-by: Samuel Rawlins --- pkg/nnbd_migration/test/api_test.dart | 58 +++++++++++++-------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/pkg/nnbd_migration/test/api_test.dart b/pkg/nnbd_migration/test/api_test.dart index 70abd3d5c2074..299408d615a01 100644 --- a/pkg/nnbd_migration/test/api_test.dart +++ b/pkg/nnbd_migration/test/api_test.dart @@ -2308,15 +2308,15 @@ main() { await _checkSingleFileChanges(content, expected); } - Future test_extension_null_check_non_nullable_method() async { + Future test_extension_null_check_non_nullable_binary() async { var content = ''' class C {} extension E on C/*!*/ { - void m() {} + void operator+(int other) {} } void f(C c, bool b) { if (b) { - c.m(); + c + 0; } } void g() => f(null, false); @@ -2324,11 +2324,11 @@ void g() => f(null, false); var expected = ''' class C {} extension E on C { - void m() {} + void operator+(int other) {} } void f(C? c, bool b) { if (b) { - c!.m(); + c! + 0; } } void g() => f(null, false); @@ -2336,27 +2336,27 @@ void g() => f(null, false); await _checkSingleFileChanges(content, expected); } - Future test_extension_null_check_non_nullable_binary() async { + Future test_extension_null_check_non_nullable_generic() async { var content = ''' class C {} -extension E on C/*!*/ { - void operator+(int other) {} +extension E on T/*!*/ { + void m() {} } void f(C c, bool b) { if (b) { - c + 0; + c.m(); } } void g() => f(null, false); '''; var expected = ''' class C {} -extension E on C { - void operator+(int other) {} +extension E on T { + void m() {} } void f(C? c, bool b) { if (b) { - c! + 0; + c!.m(); } } void g() => f(null, false); @@ -2364,15 +2364,15 @@ void g() => f(null, false); await _checkSingleFileChanges(content, expected); } - Future test_extension_null_check_non_nullable_prefix() async { + Future test_extension_null_check_non_nullable_index() async { var content = ''' class C {} extension E on C/*!*/ { - void operator-() {} + void operator[](int index) {} } void f(C c, bool b) { if (b) { - -c; + c[0]; } } void g() => f(null, false); @@ -2380,11 +2380,11 @@ void g() => f(null, false); var expected = ''' class C {} extension E on C { - void operator-() {} + void operator[](int index) {} } void f(C? c, bool b) { if (b) { - -c!; + c![0]; } } void g() => f(null, false); @@ -2392,15 +2392,15 @@ void g() => f(null, false); await _checkSingleFileChanges(content, expected); } - Future test_extension_null_check_non_nullable_index() async { + Future test_extension_null_check_non_nullable_method() async { var content = ''' class C {} extension E on C/*!*/ { - void operator[](int index) {} + void m() {} } void f(C c, bool b) { if (b) { - c[0]; + c.m(); } } void g() => f(null, false); @@ -2408,11 +2408,11 @@ void g() => f(null, false); var expected = ''' class C {} extension E on C { - void operator[](int index) {} + void m() {} } void f(C? c, bool b) { if (b) { - c![0]; + c!.m(); } } void g() => f(null, false); @@ -2420,27 +2420,27 @@ void g() => f(null, false); await _checkSingleFileChanges(content, expected); } - Future test_extension_null_check_non_nullable_generic() async { + Future test_extension_null_check_non_nullable_prefix() async { var content = ''' class C {} -extension E on T/*!*/ { - void m() {} +extension E on C/*!*/ { + void operator-() {} } void f(C c, bool b) { if (b) { - c.m(); + -c; } } void g() => f(null, false); '''; var expected = ''' class C {} -extension E on T { - void m() {} +extension E on C { + void operator-() {} } void f(C? c, bool b) { if (b) { - c!.m(); + -c!; } } void g() => f(null, false); From 9a83a75a10138066233bcd655f99285c2a2b6fed Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Thu, 3 Jun 2021 17:39:47 +0000 Subject: [PATCH 06/11] Migration: fix handling of `/*!*/` comments in is/as expressions. Previously, the migration tool had a small amount of special case logic in `EdgeBuilder._dispatch` to ensure that if an `as` expression's type was annotated with `/*!*/`, that `/*!*/` wouldn't be misinterpreted as also applying to the `as` expression itself (causing a `!` operator to be inserted). This logic wasn't sufficiently general; it failed to handle `is` expressions (which can run into the same issue), and it failed to handle situations where the `is` or `as` expression was at the end of a larger expression (e.g. `y = x as T/*!*/;`). This change removes the special case logic and replaces it with a more general-purpose mechanism: when visiting any type name, if it ends in a `/*!*/`, the edge builder stores information about it in the `_nullCheckHints` field. This ensures that when visiting an expression ending in that type name, the `/*!*/` will be ignored (since `EdgeBuilder._handleNullCheckHint` skips any hints that are already stored in that field). This ensures that we properly handle both `is` and `as` expressions, as well as larger expressions that contain them. Change-Id: I2b902ef809b4cc5eb8b493fa4405c0f0c8c10a96 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202261 Reviewed-by: Samuel Rawlins Reviewed-by: Konstantin Shcheglov Commit-Queue: Paul Berry --- pkg/nnbd_migration/lib/src/edge_builder.dart | 13 ++++-- pkg/nnbd_migration/test/api_test.dart | 48 ++++++++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/pkg/nnbd_migration/lib/src/edge_builder.dart b/pkg/nnbd_migration/lib/src/edge_builder.dart index 316b4b289b240..3ae06c19f945e 100644 --- a/pkg/nnbd_migration/lib/src/edge_builder.dart +++ b/pkg/nnbd_migration/lib/src/edge_builder.dart @@ -1912,6 +1912,13 @@ class EdgeBuilder extends GeneralizingAstVisitor } } typeName.visitChildren(this); + // If the type name is followed by a `/*!*/` comment, it is considered to + // apply to the type and not to the "as" expression. In order to prevent + // a future call to _handleNullCheck from interpreting it as applying to + // the "as" expression, we need to store the `/*!*/` comment in + // _nullCheckHints. + var token = typeName.endToken; + _nullCheckHints[token] = getPostfixHint(token); typeNameVisited( typeName); // Note this has been visited to TypeNameTracker. return null; @@ -2264,11 +2271,7 @@ class EdgeBuilder extends GeneralizingAstVisitor DecoratedType _dispatch(AstNode node, {bool skipNullCheckHint = false}) { try { var type = node?.accept(this); - if (!skipNullCheckHint && - node is Expression && - // A /*!*/ hint following an AsExpression should be interpreted as a - // nullability hint for the type, not a null-check hint. - node is! AsExpression) { + if (!skipNullCheckHint && node is Expression) { type = _handleNullCheckHint(node, type); } return type; diff --git a/pkg/nnbd_migration/test/api_test.dart b/pkg/nnbd_migration/test/api_test.dart index 299408d615a01..19248f954f35d 100644 --- a/pkg/nnbd_migration/test/api_test.dart +++ b/pkg/nnbd_migration/test/api_test.dart @@ -251,6 +251,54 @@ void main() { await _checkSingleFileChanges(content, expected); } + Future test_ambiguous_bang_hint_after_as() async { + var content = ''' +T f(Object/*?*/ x) => x as T/*!*/; +'''; + // The `/*!*/` is considered to apply to the type `T`, not to the expression + // `x as T`, so we shouldn't produce `(x as T)!`. + var expected = ''' +T f(Object? x) => x as T; +'''; + await _checkSingleFileChanges(content, expected); + } + + Future test_ambiguous_bang_hint_after_as_assigned() async { + var content = ''' +T f(Object/*?*/ x, T/*!*/ y) => y = x as T/*!*/; +'''; + // The `/*!*/` is considered to apply to the type `T`, not to the expression + // `y = x as T`, so we shouldn't produce `(y = x as T)!`. + var expected = ''' +T f(Object? x, T y) => y = x as T; +'''; + await _checkSingleFileChanges(content, expected); + } + + Future test_ambiguous_bang_hint_after_is() async { + var content = ''' +bool f(Object/*?*/ x) => x is T/*!*/; +'''; + // The `/*!*/` is considered to apply to the type `T`, not to the expression + // `x is T`, so we shouldn't produce `(x is T)!`. + var expected = ''' +bool f(Object? x) => x is T; +'''; + await _checkSingleFileChanges(content, expected); + } + + Future test_ambiguous_bang_hint_after_is_conditional() async { + var content = ''' +dynamic f(Object/*?*/ x, dynamic y) => y ? y : x is T/*!*/; +'''; + // The `/*!*/` is considered to apply to the type `T`, not to the expression + // `y ? y : x is T`, so we shouldn't produce `(y ? y : x is T)!`. + var expected = ''' +dynamic f(Object? x, dynamic y) => y ? y : x is T; +'''; + await _checkSingleFileChanges(content, expected); + } + Future test_ambiguous_closure_parameter_in_local_variable() async { var content = ''' Object f(Object Function(T) callback, Object obj) => 0; From 5a26d86753c22978e21fb7161444f8482bfc8f0f Mon Sep 17 00:00:00 2001 From: Joshua Litt Date: Thu, 3 Jun 2021 17:59:18 +0000 Subject: [PATCH 07/11] Revert "Reland "[dart2js] Only emit holders into a part file if they are used."" This reverts commit 92bfa5acfb89235e96c42f4ef130bbb450112f14. Reason for revert: Breaks for no holders in an output unit. Original change's description: > Reland "[dart2js] Only emit holders into a part file if they are used." > > This is a reland of 7af28d8274dabaef05fb351eb1fbdaf7b56aa61a > > Original change's description: > > [dart2js] Only emit holders into a part file if they are used. > > > > Change-Id: Ie1eb4dcf71e30002e3030603f201e2b6a97d4182 > > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200483 > > Commit-Queue: Joshua Litt > > Reviewed-by: Stephen Adams > > Change-Id: I06966217925a7d17615c80a5534d58f5095e579b > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201985 > Reviewed-by: Stephen Adams > Commit-Queue: Joshua Litt TBR=sra@google.com,joshualitt@google.com Change-Id: I918f3f7b496d3051046ac13dde37a8f1fbd9e892 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202304 Reviewed-by: Joshua Litt Commit-Queue: Joshua Litt --- .../deferred_holder_expression.dart | 459 +++++++----------- pkg/compiler/lib/src/js_backend/namer.dart | 5 +- .../startup_emitter/fragment_emitter.dart | 30 +- .../startup_emitter/fragment_merger.dart | 12 +- .../startup_emitter/model_emitter.dart | 7 +- 5 files changed, 192 insertions(+), 321 deletions(-) diff --git a/pkg/compiler/lib/src/js_backend/deferred_holder_expression.dart b/pkg/compiler/lib/src/js_backend/deferred_holder_expression.dart index 572d46830ebff..53980711a84f6 100644 --- a/pkg/compiler/lib/src/js_backend/deferred_holder_expression.dart +++ b/pkg/compiler/lib/src/js_backend/deferred_holder_expression.dart @@ -9,7 +9,6 @@ import '../elements/entities.dart'; import '../js/js.dart' as js; import '../serialization/serialization.dart'; import '../util/util.dart'; -import '../js_emitter/model.dart' show Fragment; import 'namer.dart'; @@ -18,7 +17,7 @@ import 'namer.dart'; // for the entity referenced in the DeferredHolderExpression. enum DeferredHolderExpressionKind { globalObjectForStaticState, - globalObjectForConstant, + globalObjectForConstants, globalObjectForLibrary, globalObjectForClass, globalObjectForMember, @@ -35,15 +34,19 @@ class DeferredHolderExpression extends js.DeferredExpression static const String tag = 'deferred-holder-expression'; final DeferredHolderExpressionKind kind; - final Object data; + final Entity entity; js.Expression _value; @override final js.JavaScriptNodeSourceInformation sourceInformation; - DeferredHolderExpression(this.kind, this.data) : sourceInformation = null; + DeferredHolderExpression(this.kind, this.entity) : sourceInformation = null; DeferredHolderExpression._( - this.kind, this.data, this._value, this.sourceInformation); + this.kind, this.entity, this._value, this.sourceInformation); + factory DeferredHolderExpression.forConstants() { + return DeferredHolderExpression( + DeferredHolderExpressionKind.globalObjectForConstants, null); + } factory DeferredHolderExpression.forStaticState() { return DeferredHolderExpression( @@ -53,26 +56,24 @@ class DeferredHolderExpression extends js.DeferredExpression factory DeferredHolderExpression.readFromDataSource(DataSource source) { source.begin(tag); var kind = source.readEnum(DeferredHolderExpressionKind.values); - Object data; + Entity entity; switch (kind) { case DeferredHolderExpressionKind.globalObjectForLibrary: - data = source.readLibrary(); + entity = source.readLibrary(); break; case DeferredHolderExpressionKind.globalObjectForClass: - data = source.readClass(); + entity = source.readClass(); break; case DeferredHolderExpressionKind.globalObjectForMember: - data = source.readMember(); - break; - case DeferredHolderExpressionKind.globalObjectForConstant: - data = source.readConstant(); + entity = source.readMember(); break; case DeferredHolderExpressionKind.globalObjectForStaticState: + case DeferredHolderExpressionKind.globalObjectForConstants: // no entity. break; } source.end(tag); - return DeferredHolderExpression(kind, data); + return DeferredHolderExpression(kind, entity); } void writeToDataSink(DataSink sink) { @@ -80,18 +81,16 @@ class DeferredHolderExpression extends js.DeferredExpression sink.writeEnum(kind); switch (kind) { case DeferredHolderExpressionKind.globalObjectForLibrary: - sink.writeLibrary(data); + sink.writeLibrary(entity); break; case DeferredHolderExpressionKind.globalObjectForClass: - sink.writeClass(data); + sink.writeClass(entity); break; case DeferredHolderExpressionKind.globalObjectForMember: - sink.writeMember(data); - break; - case DeferredHolderExpressionKind.globalObjectForConstant: - sink.writeConstant(data); + sink.writeMember(entity); break; case DeferredHolderExpressionKind.globalObjectForStaticState: + case DeferredHolderExpressionKind.globalObjectForConstants: // no entity. break; } @@ -117,7 +116,8 @@ class DeferredHolderExpression extends js.DeferredExpression js.JavaScriptNodeSourceInformation newSourceInformation) { if (newSourceInformation == sourceInformation) return this; if (newSourceInformation == null) return this; - return DeferredHolderExpression._(kind, data, _value, newSourceInformation); + return DeferredHolderExpression._( + kind, entity, _value, newSourceInformation); } @override @@ -125,7 +125,7 @@ class DeferredHolderExpression extends js.DeferredExpression @override int get hashCode { - return Hashing.objectsHash(kind, data); + return Hashing.objectsHash(kind, entity); } @override @@ -133,13 +133,13 @@ class DeferredHolderExpression extends js.DeferredExpression if (identical(this, other)) return true; return other is DeferredHolderExpression && kind == other.kind && - data == other.data; + entity == other.entity; } @override String toString() { - StringBuffer sb = StringBuffer(); - sb.write('DeferredHolderExpression(kind=$kind,data=$data,'); + StringBuffer sb = new StringBuffer(); + sb.write('DeferredHolderExpression(kind=$kind,entity=$entity,'); sb.write('value=$_value)'); return sb.toString(); } @@ -205,43 +205,40 @@ class DeferredHolderParameter extends js.Expression implements js.Parameter { @override String toString() { - StringBuffer sb = StringBuffer(); + StringBuffer sb = new StringBuffer(); sb.write('DeferredHolderParameter(name=$_name)'); return sb.toString(); } } enum DeferredHolderResourceKind { - mainFragment, - deferredFragment, + declaration, + update, } /// A [DeferredHolderResource] is a deferred JavaScript statement determined by -/// the finalization of holders. Each fragment contains one -/// [DeferredHolderResource]. The actual [Statement] contained with the +/// the finalization of holders. It is the injection point for data or +/// code to holders. The actual [Statement] contained with the /// [DeferredHolderResource] will be determined by the /// [DeferredHolderResourceKind]. These [Statement]s differ considerably -/// depending on where they are used in the AST. This class is created by the -/// fragment emitter so does not need to support serialization. +/// depending on where they are used in the AST. This class does not support +/// serialization. class DeferredHolderResource extends js.DeferredStatement implements js.AstContainer { DeferredHolderResourceKind kind; - // Each resource has a distinct name. - String name; - List fragments; Map> holderCode; + bool initializeEmptyHolders; js.Statement _statement; @override final js.JavaScriptNodeSourceInformation sourceInformation; - DeferredHolderResource(this.kind, this.name, this.fragments, this.holderCode) + DeferredHolderResource(this.kind, + {this.holderCode: const {}, this.initializeEmptyHolders: false}) : sourceInformation = null; - DeferredHolderResource._(this.kind, this.name, this.fragments, - this.holderCode, this._statement, this.sourceInformation); - - bool get isMainFragment => kind == DeferredHolderResourceKind.mainFragment; + DeferredHolderResource._(this.kind, this.holderCode, + this.initializeEmptyHolders, this._statement, this.sourceInformation); set statement(js.Statement statement) { assert(!isFinalized && statement != null); @@ -262,7 +259,7 @@ class DeferredHolderResource extends js.DeferredStatement js.JavaScriptNodeSourceInformation newSourceInformation) { if (newSourceInformation == sourceInformation) return this; if (newSourceInformation == null) return this; - return DeferredHolderResource._(kind, this.name, this.fragments, holderCode, + return DeferredHolderResource._(kind, holderCode, initializeEmptyHolders, _statement, newSourceInformation); } @@ -280,87 +277,41 @@ class DeferredHolderResource extends js.DeferredStatement } } -const String mainResourceName = 'MAIN'; - abstract class DeferredHolderExpressionFinalizer { /// Collects DeferredHolderExpressions from the JavaScript - /// AST [code] and associates it with [resourceName]. - void addCode(String resourceName, js.Node code); + /// AST [code]; + void addCode(js.Node code); /// Performs analysis on all collected DeferredHolderExpression nodes /// finalizes the values to expressions to access the holders. void finalize(); } -/// An abstraction representing a [Holder] object, which will contain some -/// portion of the programs code. -class Holder { - final String name; - final Map refCountPerResource = {}; - final Map> propertiesPerResource = {}; - int _index; - int _hashCode; - - Holder(this.name); - - int refCount(String resource) => refCountPerResource[resource]; - - void registerUse(String resource) { - refCountPerResource.update(resource, (count) => count + 1, - ifAbsent: () => 0); - } - - void registerUpdate(String resource, List properties) { - (propertiesPerResource[resource] ??= []).addAll(properties); - registerUse(resource); - } - - int get index { - assert(_index != null); - return _index; - } - - set index(int newIndex) { - assert(_index == null); - _index = newIndex; - } - - @override - bool operator ==(that) { - return that is Holder && name == that.name; - } - - @override - int get hashCode { - return _hashCode ??= Hashing.objectsHash(name); - } -} - /// [DeferredHolderExpressionFinalizerImpl] finalizes /// [DeferredHolderExpression]s, [DeferredHolderParameter]s, /// [DeferredHolderResource]s, [DeferredHolderResourceExpression]s. class DeferredHolderExpressionFinalizerImpl implements DeferredHolderExpressionFinalizer { _DeferredHolderExpressionCollectorVisitor _visitor; - final Map> holderReferences = {}; + final List holderReferences = []; final List holderParameters = []; final List holderResources = []; - final Map> holdersPerResource = {}; - final Map holderMap = {}; + final Set _uniqueHolders = {}; + final List _holders = []; + final Map _entityMap = {}; final JCommonElements _commonElements; - DeferredHolderResource mainHolderResource; DeferredHolderExpressionFinalizerImpl(this._commonElements) { _visitor = _DeferredHolderExpressionCollectorVisitor(this); } @override - void addCode(String resourceName, js.Node code) { - _visitor.setResourceNameAndVisit(resourceName, code); + void addCode(js.Node code) { + code.accept(_visitor); } final List userGlobalObjects = - List.from(Namer.reservedGlobalObjectNames) + new List.from(Namer.reservedGlobalObjectNames) ..remove('C') ..remove('H') ..remove('J') @@ -368,7 +319,7 @@ class DeferredHolderExpressionFinalizerImpl ..remove('W'); /// Returns the [reservedGlobalObjectNames] for [library]. - String globalObjectNameForLibrary(LibraryEntity library) { + String globalObjectForLibrary(LibraryEntity library) { if (library == _commonElements.interceptorsLibrary) return 'J'; Uri uri = library.canonicalUri; if (uri.scheme == 'dart') { @@ -390,285 +341,236 @@ class DeferredHolderExpressionFinalizerImpl return element.isField; } - String globalObjectNameForMember(MemberEntity entity) { + String globalObjectForMember(MemberEntity entity) { if (_isPropertyOfStaticStateHolder(entity)) { - return globalObjectNameForStaticState(); + return globalObjectForStaticState(); } else { - return globalObjectNameForLibrary(entity.library); + return globalObjectForLibrary(entity.library); } } - String globalObjectNameForClass(ClassEntity entity) { - return globalObjectNameForLibrary(entity.library); + String globalObjectForClass(ClassEntity entity) { + return globalObjectForLibrary(entity.library); } - final Holder globalObjectForStaticState = - Holder(globalObjectNameForStaticState()); - - static String globalObjectNameForStaticState() => r'$'; + String globalObjectForStaticState() => r'$'; - String globalObjectNameForConstants() => 'C'; + String globalObjectForConstants() => 'C'; - String globalObjectNameForEntity(Entity entity) { + String globalObjectForEntity(Entity entity) { if (entity is MemberEntity) { - return globalObjectNameForMember(entity); + return globalObjectForMember(entity); } else if (entity is ClassEntity) { - return globalObjectNameForLibrary(entity.library); + return globalObjectForLibrary(entity.library); } else { assert(entity is LibraryEntity); - return globalObjectNameForLibrary(entity); + return globalObjectForLibrary(entity); } } - Holder holderNameToHolder(String holderKey) { - if (holderKey == globalObjectNameForStaticState()) { - return globalObjectForStaticState; - } else { - return holderMap[holderKey]; - } - } - - Holder globalObjectForEntity(Entity entity) { - return holderNameToHolder(globalObjectNameForEntity(entity)); - } - - /// Registers a [holder] use within a given [resource], if [properties] are - /// provided then it is assumed this is an update to a holder. - void registerHolderUseOrUpdate(String resourceName, String holderName, - {List properties}) { - // For simplicity, we don't currently track the static state holder per - // resource. - if (holderName == globalObjectNameForStaticState()) return; - Holder holder = holderMap[holderName] ??= Holder(holderName); - if (properties == null) { - holder.registerUse(resourceName); - } else { - holder.registerUpdate(resourceName, properties); + /// Registers an [Entity] with a specific [holder]. + void registerHolderUse(String holder, Entity entity) { + if (_uniqueHolders.add(holder)) _holders.add(holder); + if (entity != null) { + assert(!_entityMap.containsKey(entity) || _entityMap[entity] == holder); + _entityMap[entity] = holder; } - (holdersPerResource[resourceName] ??= {}).add(holder); } - /// Returns a key to a global object for a given [Object] based on the + /// Returns a global object for a given [Entity] based on the /// [DeferredHolderExpressionKind]. - String kindToHolderName(DeferredHolderExpressionKind kind, Object data) { + String kindToHolder(DeferredHolderExpressionKind kind, Entity entity) { switch (kind) { case DeferredHolderExpressionKind.globalObjectForLibrary: - return globalObjectNameForLibrary(data); + return globalObjectForLibrary(entity); case DeferredHolderExpressionKind.globalObjectForClass: - return globalObjectNameForClass(data); + return globalObjectForClass(entity); case DeferredHolderExpressionKind.globalObjectForMember: - return globalObjectNameForMember(data); - case DeferredHolderExpressionKind.globalObjectForConstant: - return globalObjectNameForConstants(); + return globalObjectForMember(entity); + case DeferredHolderExpressionKind.globalObjectForConstants: + return globalObjectForConstants(); case DeferredHolderExpressionKind.globalObjectForStaticState: - return globalObjectNameForStaticState(); + return globalObjectForStaticState(); } throw UnsupportedError("Unreachable"); } - /// Returns a global object for a given [Object] based on the - /// [DeferredHolderExpressionKind]. - Holder kindToHolder(DeferredHolderExpressionKind kind, Object data) { - return holderNameToHolder(kindToHolderName(kind, data)); - } - - /// Finalizes [DeferredHolderParameter]s. - void finalizeParameters() { - for (var parameter in holderParameters) { - if (parameter.isFinalized) continue; - parameter.name = globalObjectNameForStaticState(); - } - } - - /// Finalizes all of the [DeferredHolderExpression]s associated with a - /// [DeferredHolderResource]. - void finalizeReferences(DeferredHolderResource resource) { - if (!holderReferences.containsKey(resource.name)) return; - for (var reference in holderReferences[resource.name]) { + /// Finalizes [DeferredHolderExpression]s [DeferredHolderParameter]s. + void finalizeReferences() { + // Finalize [DeferredHolderExpression]s and registers holder usage. + for (var reference in holderReferences) { if (reference.isFinalized) continue; - String holder = kindToHolder(reference.kind, reference.data).name; + Entity entity = reference.entity; + String holder = kindToHolder(reference.kind, entity); js.Expression value = js.VariableUse(holder); + registerHolderUse(holder, entity); reference.value = value.withSourceInformation(reference.sourceInformation); } - } - /// Registers all of the holders used in the entire program. - void registerHolders() { - // Register all holders used in all [DeferredHolderResource]s. - for (var resource in holderResources) { - resource.holderCode.forEach((entity, properties) { - String holderName = globalObjectNameForEntity(entity); - registerHolderUseOrUpdate(resource.name, holderName, - properties: properties); - }); + // Finalize [DeferredHolderParameter]s. + for (var parameter in holderParameters) { + if (parameter.isFinalized) continue; + parameter.name = globalObjectForStaticState(); } - - // Register all holders used in [DeferredHolderReference]s. - holderReferences.forEach((resource, references) { - for (var reference in references) { - String holderName = kindToHolderName(reference.kind, reference.data); - registerHolderUseOrUpdate(resource, holderName); - } - }); } - /// Returns an [Iterable] containing all of the holders used within a - /// given [DeferredHolderResource]except the static state holder (if any). - Iterable nonStaticStateHolders(DeferredHolderResource resource) { - return holdersPerResource[resource.name] ?? []; + /// Registers all of the holders used by a given [DeferredHolderResource]. + void registerHolders(DeferredHolderResource resource) { + for (var entity in resource.holderCode.keys) { + var holder = globalObjectForEntity(entity); + registerHolderUse(holder, entity); + } } - /// Returns an [Iterable] containing all of the holders used within a - /// given [DeferredHolderResource] except the static state holder. - Iterable get allNonStaticStateHolders { - return holderMap.values; + /// Returns a [List] containing all of the holders except the static + /// state holder. + List get nonStaticStateHolders { + return _holders + .where((holder) => holder != globalObjectForStaticState()) + .toList(growable: false); } - /// Generates code to declare holders for a given [resourceName]. - HolderInitCode declareHolders(String resourceName, Iterable holders, - {bool initializeEmptyHolders = false}) { - // Create holder initialization code. If there are no properties - // associated with a given holder in this specific [DeferredHolderResource] - // then it will be omitted. However, in some cases, i.e. the main output - // unit, we still want to declare the holder with an empty object literal - // which will be filled in later by another [DeferredHolderResource], i.e. - // in a specific deferred fragment. The generated code looks like this: + /// Generates code to declare holders. + HolderCode declareHolders(DeferredHolderResource resource) { + // Collect all holders except the static state holder. Then, create a map of + // holder to list of properties which are associated with that holder, but + // only with respect to a given [DeferredHolderResource]. Each fragment will + // have its own [DeferredHolderResource] and associated code. + Map> codePerHolder = {}; + final holders = nonStaticStateHolders; + for (var holder in holders) { + codePerHolder[holder] = []; + } + + final holderCode = resource.holderCode; + holderCode.forEach((entity, properties) { + assert(_entityMap.containsKey(entity)); + var holder = _entityMap[entity]; + assert(codePerHolder.containsKey(holder)); + codePerHolder[holder].addAll(properties); + }); + + // Create holder initialization code based on the [codePerHolder]. If there + // are no properties associated with a given holder in this specific + // [DeferredHolderResource] then it will be omitted. However, in some cases, + // i.e. the main output unit, we still want to declare the holder with an + // empty object literal which will be filled in later by another + // [DeferredHolderResource], i.e. in a specific deferred fragment. + // The generated code looks like this: // // { // var H = {...}, ..., G = {...}; + // var holders = [ H, ..., G ]; // Main unit only. // } - List activeHolders = []; + List activeHolders = []; List holderInitializations = []; for (var holder in holders) { - var holderName = holder.name; - List properties = - holder.propertiesPerResource[resourceName] ?? []; + List properties = codePerHolder[holder]; if (properties.isEmpty) { holderInitializations.add(js.VariableInitialization( - js.VariableDeclaration(holderName, allowRename: false), - initializeEmptyHolders ? js.ObjectInitializer(properties) : null)); + js.VariableDeclaration(holder, allowRename: false), + resource.initializeEmptyHolders + ? js.ObjectInitializer(properties) + : null)); } else { activeHolders.add(holder); holderInitializations.add(js.VariableInitialization( - js.VariableDeclaration(holderName, allowRename: false), + js.VariableDeclaration(holder, allowRename: false), js.ObjectInitializer(properties))); } } - // Create statement to initialize holders. - var initStatement = js.ExpressionStatement( - js.VariableDeclarationList(holderInitializations, indentSplits: false)); - return HolderInitCode(holders, activeHolders, initStatement); + List statements = []; + statements.add(js.ExpressionStatement(js.VariableDeclarationList( + holderInitializations, + indentSplits: false))); + if (resource.initializeEmptyHolders) { + statements.add(js.js.statement( + 'var holders = #', + js.ArrayInitializer(holders + .map((holder) => js.VariableUse(holder)) + .toList(growable: false)))); + } + return HolderCode(activeHolders, statements); } /// Finalizes [resource] to code that updates holders. [resource] must be in /// the AST of a deferred fragment. void updateHolders(DeferredHolderResource resource) { - final holderCode = - declareHolders(resource.name, nonStaticStateHolders(resource)); + // Declare holders. + final holderCode = declareHolders(resource); // Set names if necessary on deferred holders list. js.Expression deferredHoldersList = js.ArrayInitializer(holderCode .activeHolders - .map((holder) => js.js("#", holder.name)) + .map((holder) => js.js("#", holder)) .toList(growable: false)); js.Statement setNames = js.js.statement( 'hunkHelpers.setFunctionNamesIfNecessary(#deferredHoldersList)', {'deferredHoldersList': deferredHoldersList}); // Update holder assignments. - List updateHolderAssignments = [ - holderCode.statement, - setNames - ]; - for (var holder in holderCode.allHolders) { - var holderName = holder.name; - var holderIndex = js.number(holder.index); + final holders = nonStaticStateHolders; + List updateHolderAssignments = [setNames]; + for (int i = 0; i < holders.length; i++) { + var holder = holders[i]; if (holderCode.activeHolders.contains(holder)) { updateHolderAssignments.add(js.js.statement( '#holder = hunkHelpers.updateHolder(holdersList[#index], #holder)', - {'index': holderIndex, 'holder': js.VariableUse(holderName)})); + {'index': js.number(i), 'holder': js.VariableUse(holder)})); } else { // TODO(sra): Change declaration followed by assignments to declarations // with initialization. updateHolderAssignments.add(js.js.statement( '#holder = holdersList[#index]', - {'index': holderIndex, 'holder': js.VariableUse(holderName)})); + {'index': js.number(i), 'holder': js.VariableUse(holder)})); } } // Create a single block of all statements. - resource.statement = js.Block(updateHolderAssignments); + List statements = holderCode.statements + .followedBy(updateHolderAssignments) + .toList(growable: false); + resource.statement = js.Block(statements); } - /// Declares all holders in the [DeferredHolderResource] representing the main - /// fragment. - void declareHoldersInMainResource() { - // Declare holders in main output unit. - var holders = allNonStaticStateHolders; - var holderCode = declareHolders(mainHolderResource.name, holders, - initializeEmptyHolders: true); - - // Create holder uses and init holder indices. - List holderUses = []; - int i = 0; - for (var holder in holders) { - holder.index = i++; - holderUses.add(js.VariableUse(holder.name)); + /// Allocates all [DeferredHolderResource]s and + /// [DeferredHolderResourceExpression]s. + void allocateResources() { + // First ensure all holders used in all [DeferredHolderResource]s have been + // allocated. + for (var resource in holderResources) { + registerHolders(resource); } - - // Create holders array statement. - // { - // var holders = [ H, ..., G ]; - // } - var holderArray = - js.js.statement('var holders = #', js.ArrayInitializer(holderUses)); - - mainHolderResource.statement = - js.Block([holderCode.statement, holderArray]); - } - - /// Allocates all [DeferredHolderResource]s and finalizes the associated - /// [DeferredHolderExpression]s. - void allocateResourcesAndFinalizeReferences() { - // First finalize all holders in the main output unit. - declareHoldersInMainResource(); + _holders.sort(); // Next finalize all [DeferredHolderResource]s. for (var resource in holderResources) { switch (resource.kind) { - case DeferredHolderResourceKind.mainFragment: - // There should only be one main resource and at this point it - // should have already been finalized. - assert(mainHolderResource == resource && resource.isFinalized); + case DeferredHolderResourceKind.declaration: + var holderCode = declareHolders(resource); + resource.statement = js.Block(holderCode.statements); break; - case DeferredHolderResourceKind.deferredFragment: + case DeferredHolderResourceKind.update: updateHolders(resource); break; } - finalizeReferences(resource); } } @override void finalize() { - registerHolders(); - finalizeParameters(); - allocateResourcesAndFinalizeReferences(); + finalizeReferences(); + allocateResources(); } - void _registerDeferredHolderExpression( - String resourceName, DeferredHolderExpression node) { - (holderReferences[resourceName] ??= []).add(node); + void _registerDeferredHolderExpression(DeferredHolderExpression node) { + holderReferences.add(node); } void _registerDeferredHolderResource(DeferredHolderResource node) { - if (node.isMainFragment) { - assert(mainHolderResource == null); - mainHolderResource = node; - } holderResources.add(node); } @@ -684,17 +586,10 @@ class DeferredHolderExpressionFinalizerImpl /// The state is kept in the finalizer so that this scan could be extended to /// look for other deferred expressions in one pass. class _DeferredHolderExpressionCollectorVisitor extends js.BaseVisitor { - String resourceName; final DeferredHolderExpressionFinalizerImpl _finalizer; _DeferredHolderExpressionCollectorVisitor(this._finalizer); - void setResourceNameAndVisit(String resourceName, js.Node code) { - this.resourceName = resourceName; - code.accept(this); - this.resourceName = null; - } - @override void visitNode(js.Node node) { assert(node is! DeferredHolderExpression); @@ -710,8 +605,7 @@ class _DeferredHolderExpressionCollectorVisitor extends js.BaseVisitor { @override void visitDeferredExpression(js.DeferredExpression node) { if (node is DeferredHolderExpression) { - assert(resourceName != null); - _finalizer._registerDeferredHolderExpression(resourceName, node); + _finalizer._registerDeferredHolderExpression(node); } else { visitNode(node); } @@ -736,9 +630,8 @@ class _DeferredHolderExpressionCollectorVisitor extends js.BaseVisitor { } } -class HolderInitCode { - final Iterable allHolders; - final List activeHolders; - final js.Statement statement; - HolderInitCode(this.allHolders, this.activeHolders, this.statement); +class HolderCode { + final List activeHolders; + final List statements; + HolderCode(this.activeHolders, this.statements); } diff --git a/pkg/compiler/lib/src/js_backend/namer.dart b/pkg/compiler/lib/src/js_backend/namer.dart index 5e4afbdbddb87..8503a49f09a1f 100644 --- a/pkg/compiler/lib/src/js_backend/namer.dart +++ b/pkg/compiler/lib/src/js_backend/namer.dart @@ -2071,9 +2071,8 @@ abstract class ModularNamer { FixedNames get fixedNames; /// Returns a variable use for accessing constants. - jsAst.Expression globalObjectForConstant(ConstantValue constant) { - return DeferredHolderExpression( - DeferredHolderExpressionKind.globalObjectForConstant, constant); + jsAst.Expression globalObjectForConstants() { + return DeferredHolderExpression.forConstants(); } /// Returns a variable use for accessing static state. diff --git a/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart b/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart index a4249ee26d3c7..bea472aebf39b 100644 --- a/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart +++ b/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart @@ -679,7 +679,6 @@ class FragmentEmitter { size = estimator.charCount; } var emittedOutputUnit = EmittedOutputUnit( - fragment, fragment.outputUnit, fragment.libraries, classPrototypes, @@ -705,10 +704,9 @@ class FragmentEmitter { // Emit holder code. var holderCode = emitHolderCode(fragment.libraries); var holderDeclaration = DeferredHolderResource( - DeferredHolderResourceKind.mainFragment, - mainResourceName, - [fragment], - holderCode); + DeferredHolderResourceKind.declaration, + holderCode: holderCode, + initializeEmptyHolders: true); js.Statement mainCode = js.js.statement(_mainBoilerplate, { // TODO(29455): 'hunkHelpers' displaces other names, so don't minify it. 'hunkHelpers': js.VariableDeclaration('hunkHelpers', allowRename: false), @@ -760,7 +758,7 @@ class FragmentEmitter { 'call2selector': js.quoteName(call2Name) }); // We assume emitMainFragment will be the last piece of code we emit. - finalizeCode(mainResourceName, mainCode, holderCode, finalizeHolders: true); + finalizeCode(mainCode, holderCode, finalizeHolders: true); return mainCode; } @@ -780,12 +778,9 @@ class FragmentEmitter { return null; } - var resourceName = fragment.canonicalOutputUnit.name; var updateHolders = DeferredHolderResource( - DeferredHolderResourceKind.deferredFragment, - resourceName, - fragment.fragments, - holderCode); + DeferredHolderResourceKind.update, + holderCode: holderCode); js.Expression code = js.js(_deferredBoilerplate, { // TODO(floitsch): don't just reference 'init'. 'embeddedGlobalsObject': new js.Parameter('init'), @@ -811,7 +806,7 @@ class FragmentEmitter { if (_options.experimentStartupFunctions) { code = js.Parentheses(code); } - finalizeCode(resourceName, code, holderCode); + finalizeCode(code, holderCode); return code; } @@ -828,8 +823,7 @@ class FragmentEmitter { /// Finalizes the code for a fragment, and optionally finalizes holders. /// Finalizing holders must be the last step of the emitter. - void finalizeCode(String resourceName, js.Node code, - Map> holderCode, + void finalizeCode(js.Node code, Map> holderCode, {bool finalizeHolders: false}) { StringReferenceFinalizer stringFinalizer = StringReferenceFinalizerImpl(_options.enableMinification); @@ -847,11 +841,7 @@ class FragmentEmitter { // per output unit, the holderFinalizer is a whole-program finalizer, // which collects deferred [Node]s from each call to `finalizeCode` // before begin finalized once for the last (main) unit. - void _addCode(js.Node code) { - _holderFinalizer.addCode(resourceName, code); - } - - addCodeToFinalizer(_addCode, code, holderCode); + addCodeToFinalizer(_holderFinalizer.addCode, code, holderCode); if (finalizeHolders) { _holderFinalizer.finalize(); } @@ -1579,7 +1569,7 @@ class FragmentEmitter { // TODO(25230): We only need to name constants that are used from function // bodies or from other constants in a different part. var assignment = js.js.statement('#.# = #', [ - _namer.globalObjectForConstant(constant.value), + _namer.globalObjectForConstants(), constant.name, _constantEmitter.generate(constant.value) ]); diff --git a/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_merger.dart b/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_merger.dart index 9fae89cef9292..89cc19c49ceb1 100644 --- a/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_merger.dart +++ b/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_merger.dart @@ -109,7 +109,6 @@ import '../model.dart'; /// sent to the client. class EmittedOutputUnit { - final Fragment fragment; final OutputUnit outputUnit; final List libraries; final js.Statement classPrototypes; @@ -125,7 +124,6 @@ class EmittedOutputUnit { final js.Statement nativeSupport; EmittedOutputUnit( - this.fragment, this.outputUnit, this.libraries, this.classPrototypes, @@ -142,7 +140,6 @@ class EmittedOutputUnit { CodeFragment toCodeFragment(Program program) { return CodeFragment( - [fragment], [outputUnit], libraries, classPrototypes, @@ -204,7 +201,6 @@ class PreFragment { return seedEmittedOutputUnit.toCodeFragment(program); } else { var seedOutputUnit = seedEmittedOutputUnit.outputUnit; - List fragments = []; List libraries = []; List outputUnits = [seedOutputUnit]; List classPrototypes = []; @@ -223,7 +219,6 @@ class PreFragment { if (seedOutputUnit != thatOutputUnit) { program.mergeOutputUnitMetadata(seedOutputUnit, thatOutputUnit); outputUnits.add(thatOutputUnit); - fragments.add(emittedOutputUnit.fragment); } libraries.addAll(emittedOutputUnit.libraries); classPrototypes.add(emittedOutputUnit.classPrototypes); @@ -239,7 +234,6 @@ class PreFragment { nativeSupport.add(emittedOutputUnit.nativeSupport); } return CodeFragment( - fragments, outputUnits, libraries, js.Block(classPrototypes), @@ -322,7 +316,6 @@ class PreFragment { } class CodeFragment { - final List fragments; final List outputUnits; final List libraries; final js.Statement classPrototypes; @@ -339,7 +332,6 @@ class CodeFragment { final js.Expression deferredTypes; CodeFragment( - this.fragments, this.outputUnits, this.libraries, this.classPrototypes, @@ -396,8 +388,6 @@ class CodeFragment { } return outputUnitStrings.join('+'); } - - OutputUnit get canonicalOutputUnit => outputUnits.first; } class FinalizedFragment { @@ -410,7 +400,7 @@ class FinalizedFragment { // TODO(joshualitt): Refactor this to more clearly disambiguate between // [OutputUnits](units of deferred merging), fragments(units of emitted code), // and files. - OutputUnit get canonicalOutputUnit => codeFragments.first.canonicalOutputUnit; + OutputUnit get canonicalOutputUnit => codeFragments.first.outputUnits.first; @override String toString() { diff --git a/pkg/compiler/lib/src/js_emitter/startup_emitter/model_emitter.dart b/pkg/compiler/lib/src/js_emitter/startup_emitter/model_emitter.dart index 8a272c6b40b64..21fe02fb99355 100644 --- a/pkg/compiler/lib/src/js_emitter/startup_emitter/model_emitter.dart +++ b/pkg/compiler/lib/src/js_emitter/startup_emitter/model_emitter.dart @@ -65,8 +65,7 @@ import '../../js_backend/deferred_holder_expression.dart' DeferredHolderExpressionFinalizerImpl, DeferredHolderParameter, DeferredHolderResource, - DeferredHolderResourceKind, - mainResourceName; + DeferredHolderResourceKind; import '../../js_backend/type_reference.dart' show TypeReferenceFinalizer, @@ -219,8 +218,8 @@ class ModelEmitter { if (isConstantInlinedOrAlreadyEmitted(value)) { return constantEmitter.generate(value); } - return js.js('#.#', - [_namer.globalObjectForConstant(value), _namer.constantName(value)]); + return js.js( + '#.#', [_namer.globalObjectForConstants(), _namer.constantName(value)]); } bool get shouldMergeFragments => _options.mergeFragmentsThreshold != null; From b9811285ac25da3b403e4579faec59c999fd6779 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Thu, 3 Jun 2021 18:18:57 +0000 Subject: [PATCH 08/11] pkg:wasm - have setup.dart output to the current package directory ...not to the directory that currently contains pkg:wasm Also DRY'd up some values and helpers Change-Id: I71292dacf089e285dd5080e9830e8a6e520c1491 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202302 Auto-Submit: Kevin Moore Reviewed-by: Liam Appelbe Commit-Queue: Liam Appelbe --- pkg/wasm/bin/setup.dart | 37 +++++++++++++++++------------------ pkg/wasm/lib/src/runtime.dart | 20 +++++++------------ pkg/wasm/lib/src/shared.dart | 22 +++++++++++++++++++++ 3 files changed, 47 insertions(+), 32 deletions(-) create mode 100644 pkg/wasm/lib/src/shared.dart diff --git a/pkg/wasm/bin/setup.dart b/pkg/wasm/bin/setup.dart index ac713b5dcd112..2497ecd9c7b88 100644 --- a/pkg/wasm/bin/setup.dart +++ b/pkg/wasm/bin/setup.dart @@ -10,9 +10,11 @@ import 'dart:convert'; import 'dart:io' hide exit; +import 'package:wasm/src/shared.dart'; + Future main(List args) async { if (args.length > 1) { - print('Usage: dart run wasm:setup [target-triple]'); + print('Usage: $invocationString [target-triple]'); exitCode = 64; // bad usage return; } @@ -34,7 +36,7 @@ Uri _getSdkDir() { // path.dirname called twice on Platform.resolvedExecutable. final exe = Uri.file(Platform.resolvedExecutable); final commonSdkDir = exe.resolve('../../dart-sdk/'); - if (Directory(commonSdkDir.path).existsSync()) { + if (FileSystemEntity.isDirectorySync(commonSdkDir.path)) { return commonSdkDir; } @@ -42,13 +44,13 @@ Uri _getSdkDir() { // SDK, and is executing dart via: // ./out/ReleaseX64/dart ... final checkedOutSdkDir = exe.resolve('../dart-sdk/'); - if (Directory(checkedOutSdkDir.path).existsSync()) { + if (FileSystemEntity.isDirectorySync(checkedOutSdkDir.path)) { return checkedOutSdkDir; } final homebrewOutSdkDir = exe.resolve('..'); final homebrewIncludeDir = homebrewOutSdkDir.resolve('include'); - if (Directory(homebrewIncludeDir.path).existsSync()) { + if (FileSystemEntity.isDirectorySync(homebrewIncludeDir.path)) { return homebrewOutSdkDir; } @@ -57,24 +59,21 @@ Uri _getSdkDir() { } Uri _getOutDir(Uri root) { - // Traverse up until we see a `.dart_tool/package_config.json` file. - do { - if (File.fromUri(root.resolve('.dart_tool/package_config.json')) - .existsSync()) { - return root.resolve('.dart_tool/wasm/'); - } - } while (root != (root = root.resolve('..'))); - throw Exception('.dart_tool/package_config.json not found'); + final pkgRoot = packageRootUri(root); + if (pkgRoot == null) { + throw Exception('$pkgConfigFile not found'); + } + return pkgRoot.resolve(wasmToolDir); } String _getOutLib(String target) { final os = RegExp(r'^.*-.*-(.*)').firstMatch(target)?.group(1) ?? ''; if (os == 'darwin' || os == 'ios') { - return 'libwasmer.dylib'; + return appleLib; } else if (os == 'windows') { return 'wasmer.dll'; } - return 'libwasmer.so'; + return linuxLib; } Future _getTargetTriple() async { @@ -116,7 +115,7 @@ Future _run(String exe, List args) async { Future _main(String target) async { final sdkDir = _getSdkDir(); final binDir = Platform.script; - final outDir = _getOutDir(binDir); + final outDir = _getOutDir(Directory.current.uri); final outLib = outDir.resolve(_getOutLib(target)).path; print('Dart SDK directory: ${sdkDir.path}'); @@ -137,14 +136,14 @@ Future _main(String target) async { '--release' ]); - final dartApiDlImplFile = - File.fromUri(sdkDir.resolve('include/internal/dart_api_dl_impl.h')); + const dartApiDlImplPath = 'include/internal/dart_api_dl_impl.h'; + + final dartApiDlImplFile = File.fromUri(sdkDir.resolve(dartApiDlImplPath)); // Hack around a bug with dart_api_dl_impl.h include path in dart_api_dl.c. if (!dartApiDlImplFile.existsSync()) { Directory(outDir.resolve('include/internal/').path) .createSync(recursive: true); - await dartApiDlImplFile - .copy(outDir.resolve('include/internal/dart_api_dl_impl.h').path); + await dartApiDlImplFile.copy(outDir.resolve(dartApiDlImplPath).path); } // Build dart_api_dl.o. diff --git a/pkg/wasm/lib/src/runtime.dart b/pkg/wasm/lib/src/runtime.dart index 40e892b5eefc0..3f9bcf0f03bd8 100644 --- a/pkg/wasm/lib/src/runtime.dart +++ b/pkg/wasm/lib/src/runtime.dart @@ -10,6 +10,7 @@ import 'dart:typed_data'; import 'package:ffi/ffi.dart'; +import 'shared.dart'; import 'wasmer_api.dart'; part 'runtime.g.dart'; @@ -99,23 +100,16 @@ class _WasiStreamIterable extends Iterable> { } String _getLibName() { - if (Platform.isMacOS) return 'libwasmer.dylib'; - if (Platform.isLinux) return 'libwasmer.so'; + if (Platform.isMacOS) return appleLib; + if (Platform.isLinux) return linuxLib; // TODO(dartbug.com/37882): Support more platforms. throw Exception('Wasm not currently supported on this platform'); } String? _getLibPathFrom(Uri root) { - // The dynamic library created by pub run wasm:setup is located relative to - // the package_config.json file, so walk up from the script directory until - // we find it. - do { - if (File.fromUri(root.resolve('.dart_tool/package_config.json')) - .existsSync()) { - return root.resolve('.dart_tool/wasm/${_getLibName()}').path; - } - } while (root != (root = root.resolve('..'))); - return null; + final pkgRoot = packageRootUri(root); + + return pkgRoot?.resolve('$wasmToolDir${_getLibName()}').path; } String _getLibPath() { @@ -123,5 +117,5 @@ String _getLibPath() { if (path != null) return path; path = _getLibPathFrom(Directory.current.uri); if (path != null) return path; - throw Exception('Wasm library not found. Did you `pub run wasm:setup`?'); + throw Exception('Wasm library not found. Did you `$invocationString`?'); } diff --git a/pkg/wasm/lib/src/shared.dart b/pkg/wasm/lib/src/shared.dart new file mode 100644 index 0000000000000..0f666b7612ac8 --- /dev/null +++ b/pkg/wasm/lib/src/shared.dart @@ -0,0 +1,22 @@ +// Copyright (c) 2020, 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. + +import 'dart:io'; + +const invocationString = 'dart run wasm:setup'; + +const pkgConfigFile = '.dart_tool/package_config.json'; +const wasmToolDir = '.dart_tool/wasm/'; + +const appleLib = 'libwasmer.dylib'; +const linuxLib = 'libwasmer.so'; + +Uri? packageRootUri(Uri root) { + do { + if (FileSystemEntity.isFileSync(root.resolve(pkgConfigFile).path)) { + return root; + } + } while (root != (root = root.resolve('..'))); + return null; +} From ad233e5ff1f591a92bd6cc6f211d1a0eb5eaf056 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Thu, 3 Jun 2021 18:23:07 +0000 Subject: [PATCH 09/11] [vm, profiler] Compact the representation of Samples. TEST=ci Change-Id: I6078479074b5681fad194d1457717878ecc0f6f2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202063 Commit-Queue: Ryan Macnak Reviewed-by: Ben Konyi --- runtime/vm/malloc_hooks_tcmalloc.cc | 4 +- runtime/vm/malloc_hooks_test.cc | 4 +- runtime/vm/malloc_hooks_unsupported.cc | 2 +- runtime/vm/profiler.cc | 41 +++------- runtime/vm/profiler.h | 106 ++++++++++++------------- 5 files changed, 68 insertions(+), 89 deletions(-) diff --git a/runtime/vm/malloc_hooks_tcmalloc.cc b/runtime/vm/malloc_hooks_tcmalloc.cc index 443285e3df1cc..438e4772c083a 100644 --- a/runtime/vm/malloc_hooks_tcmalloc.cc +++ b/runtime/vm/malloc_hooks_tcmalloc.cc @@ -4,7 +4,7 @@ #include "platform/globals.h" -#if defined(DART_USE_TCMALLOC) && !defined(PRODUCT) +#if defined(DART_USE_TCMALLOC) && defined(DEBUG) #include "vm/malloc_hooks.h" @@ -421,4 +421,4 @@ void MallocHooksState::RecordFreeHook(const void* ptr) { } // namespace dart -#endif // defined(DART_USE_TCMALLOC) && !defined(PRODUCT) +#endif // defined(DART_USE_TCMALLOC) && !defined(DEBUG) diff --git a/runtime/vm/malloc_hooks_test.cc b/runtime/vm/malloc_hooks_test.cc index 76b789ba2b9d9..8edd992f31c5d 100644 --- a/runtime/vm/malloc_hooks_test.cc +++ b/runtime/vm/malloc_hooks_test.cc @@ -4,7 +4,7 @@ #include "platform/globals.h" -#if defined(DART_USE_TCMALLOC) && !defined(PRODUCT) +#if defined(DART_USE_TCMALLOC) && defined(DEBUG) #include "platform/assert.h" #include "vm/globals.h" @@ -200,4 +200,4 @@ ISOLATE_UNIT_TEST_CASE(StackTraceMallocHookSimpleJSONTest) { }; // namespace dart -#endif // defined(DART_USE_TCMALLOC) && !defined(PRODUCT) +#endif // defined(DART_USE_TCMALLOC) && !defined(DEBUG) diff --git a/runtime/vm/malloc_hooks_unsupported.cc b/runtime/vm/malloc_hooks_unsupported.cc index bf986dc239940..d28100f63ac66 100644 --- a/runtime/vm/malloc_hooks_unsupported.cc +++ b/runtime/vm/malloc_hooks_unsupported.cc @@ -4,7 +4,7 @@ #include "platform/globals.h" -#if defined(PRODUCT) || !defined(DART_USE_TCMALLOC) +#if !defined(DEBUG) || !defined(DART_USE_TCMALLOC) #include "vm/malloc_hooks.h" diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc index 684d7f3b16fb8..51a7f20c4ef89 100644 --- a/runtime/vm/profiler.cc +++ b/runtime/vm/profiler.cc @@ -27,7 +27,6 @@ namespace dart { -static const intptr_t kSampleSize = 8; static const intptr_t kMaxSamplesPerTick = 16; DEFINE_FLAG(bool, trace_profiled_isolates, false, "Trace profiled isolates."); @@ -38,7 +37,7 @@ DEFINE_FLAG(int, "Time between profiler samples in microseconds. Minimum 50."); DEFINE_FLAG(int, max_profile_depth, - kSampleSize* kMaxSamplesPerTick, + Sample::kPCArraySizeInWords* kMaxSamplesPerTick, "Maximum number stack frames walked. Minimum 1. Maximum 255."); #if defined(USING_SIMULATOR) DEFINE_FLAG(bool, profile_vm, true, "Always collect native stack traces."); @@ -69,7 +68,6 @@ ProfilerCounters Profiler::counters_ = {}; void Profiler::Init() { // Place some sane restrictions on user controlled flags. SetSampleDepth(FLAG_max_profile_depth); - Sample::Init(); if (!FLAG_profiler) { return; } @@ -158,24 +156,9 @@ void Profiler::UpdateSamplePeriod() { SetSamplePeriod(FLAG_profile_period); } -intptr_t Sample::pcs_length_ = 0; -intptr_t Sample::instance_size_ = 0; - -void Sample::Init() { - pcs_length_ = kSampleSize; - instance_size_ = sizeof(Sample) + (sizeof(uword) * pcs_length_); // NOLINT. -} - -uword* Sample::GetPCArray() const { - return reinterpret_cast(reinterpret_cast(this) + - sizeof(*this)); -} - SampleBuffer::SampleBuffer(intptr_t capacity) { - ASSERT(Sample::instance_size() > 0); - - const intptr_t size = Utils::RoundUp(capacity * Sample::instance_size(), - VirtualMemory::PageSize()); + const intptr_t size = + Utils::RoundUp(capacity * sizeof(Sample), VirtualMemory::PageSize()); const bool kNotExecutable = false; memory_ = VirtualMemory::Allocate(size, kNotExecutable, "dart-profiler"); if (memory_ == NULL) { @@ -188,7 +171,7 @@ SampleBuffer::SampleBuffer(intptr_t capacity) { if (FLAG_trace_profiler) { OS::PrintErr("Profiler holds %" Pd " samples\n", capacity); - OS::PrintErr("Profiler sample is %" Pd " bytes\n", Sample::instance_size()); + OS::PrintErr("Profiler sample is %" Pd " bytes\n", sizeof(Sample)); OS::PrintErr("Profiler memory usage = %" Pd " bytes\n", size); } if (FLAG_sample_buffer_duration != 0) { @@ -218,9 +201,7 @@ AllocationSampleBuffer::~AllocationSampleBuffer() { Sample* SampleBuffer::At(intptr_t idx) const { ASSERT(idx >= 0); ASSERT(idx < capacity_); - intptr_t offset = idx * Sample::instance_size(); - uint8_t* samples = reinterpret_cast(samples_); - return reinterpret_cast(samples + offset); + return &samples_[idx]; } intptr_t SampleBuffer::ReserveSampleSlot() { @@ -273,7 +254,7 @@ intptr_t AllocationSampleBuffer::ReserveSampleSlotLocked() { uint8_t* samples_array_ptr = reinterpret_cast(samples_); uint8_t* free_sample_ptr = reinterpret_cast(free_sample); return static_cast((free_sample_ptr - samples_array_ptr) / - Sample::instance_size()); + sizeof(Sample)); } else if (cursor_ < static_cast(capacity_ - 1)) { return cursor_ += 1; } else { @@ -510,7 +491,7 @@ class ProfilerStackWalker : public ValueObject { return false; } ASSERT(sample_ != NULL); - if (frame_index_ == kSampleSize) { + if (frame_index_ == Sample::kPCArraySizeInWords) { Sample* new_sample = sample_buffer_->ReserveSampleAndLink(sample_); if (new_sample == NULL) { // Could not reserve new sample- mark this as truncated. @@ -520,7 +501,7 @@ class ProfilerStackWalker : public ValueObject { frame_index_ = 0; sample_ = new_sample; } - ASSERT(frame_index_ < kSampleSize); + ASSERT(frame_index_ < Sample::kPCArraySizeInWords); sample_->SetAt(frame_index_, pc); frame_index_++; total_frames_++; @@ -1573,7 +1554,7 @@ ProcessedSample* SampleBuffer::BuildProcessedSample( bool truncated = false; Sample* current = sample; while (current != NULL) { - for (intptr_t i = 0; i < kSampleSize; i++) { + for (intptr_t i = 0; i < Sample::kPCArraySizeInWords; i++) { if (current->At(i) == 0) { break; } @@ -1585,7 +1566,7 @@ ProcessedSample* SampleBuffer::BuildProcessedSample( } if (!sample->exit_frame_sample()) { - processed_sample->FixupCaller(clt, sample->pc_marker(), + processed_sample->FixupCaller(clt, /* pc_marker */ 0, sample->GetStackBuffer()); } @@ -1612,7 +1593,7 @@ Sample* SampleBuffer::Next(Sample* sample) { } ProcessedSample::ProcessedSample() - : pcs_(kSampleSize), + : pcs_(Sample::kPCArraySizeInWords), timestamp_(0), vm_tag_(0), user_tag_(0), diff --git a/runtime/vm/profiler.h b/runtime/vm/profiler.h index 215d3575706c0..6dad27485cffb 100644 --- a/runtime/vm/profiler.h +++ b/runtime/vm/profiler.h @@ -202,25 +202,25 @@ class Sample { ThreadId tid() const { return tid_; } void Clear() { + timestamp_ = 0; port_ = ILLEGAL_PORT; - pc_marker_ = 0; + tid_ = OSThread::kInvalidThreadId; for (intptr_t i = 0; i < kStackBufferSizeInWords; i++) { stack_buffer_[i] = 0; } + for (intptr_t i = 0; i < kPCArraySizeInWords; i++) { + pc_array_[i] = 0; + } vm_tag_ = VMTag::kInvalidTagId; user_tag_ = UserTags::kDefaultUserTag; - lr_ = 0; - metadata_ = 0; state_ = 0; + continuation_index_ = -1; allocation_identity_hash_ = 0; +#if defined(DART_USE_TCMALLOC) && defined(DEBUG) native_allocation_address_ = 0; native_allocation_size_bytes_ = 0; - continuation_index_ = -1; next_free_ = NULL; - uword* pcs = GetPCArray(); - for (intptr_t i = 0; i < pcs_length_; i++) { - pcs[i] = 0; - } +#endif set_head_sample(true); } @@ -233,21 +233,19 @@ class Sample { // Get stack trace entry. uword At(intptr_t i) const { ASSERT(i >= 0); - ASSERT(i < pcs_length_); - uword* pcs = GetPCArray(); - return pcs[i]; + ASSERT(i < kPCArraySizeInWords); + return pc_array_[i]; } // Set stack trace entry. void SetAt(intptr_t i, uword pc) { ASSERT(i >= 0); - ASSERT(i < pcs_length_); - uword* pcs = GetPCArray(); - pcs[i] = pc; + ASSERT(i < kPCArraySizeInWords); + pc_array_[i] = pc; } void DumpStackTrace() { - for (intptr_t i = 0; i < pcs_length_; ++i) { + for (intptr_t i = 0; i < kPCArraySizeInWords; ++i) { uintptr_t start = 0; uword pc = At(i); char* native_symbol_name = @@ -270,14 +268,6 @@ class Sample { uword user_tag() const { return user_tag_; } void set_user_tag(uword tag) { user_tag_ = tag; } - uword pc_marker() const { return pc_marker_; } - - void set_pc_marker(uword pc_marker) { pc_marker_ = pc_marker; } - - uword lr() const { return lr_; } - - void set_lr(uword link_register) { lr_ = link_register; } - bool leaf_frame_is_dart() const { return LeafFrameIsDart::decode(state_); } void set_leaf_frame_is_dart(bool leaf_frame_is_dart) { @@ -326,8 +316,8 @@ class Sample { allocation_identity_hash_ = hash; } +#if defined(DART_USE_TCMALLOC) && defined(DEBUG) uword native_allocation_address() const { return native_allocation_address_; } - void set_native_allocation_address(uword address) { native_allocation_address_ = address; } @@ -335,13 +325,22 @@ class Sample { uintptr_t native_allocation_size_bytes() const { return native_allocation_size_bytes_; } - void set_native_allocation_size_bytes(uintptr_t size) { native_allocation_size_bytes_ = size; } Sample* next_free() const { return next_free_; } void set_next_free(Sample* next_free) { next_free_ = next_free; } +#else + uword native_allocation_address() const { return 0; } + void set_native_allocation_address(uword address) { UNREACHABLE(); } + + uintptr_t native_allocation_size_bytes() const { return 0; } + void set_native_allocation_size_bytes(uintptr_t size) { UNREACHABLE(); } + + Sample* next_free() const { return nullptr; } + void set_next_free(Sample* next_free) { UNREACHABLE(); } +#endif // defined(DART_USE_TCMALLOC) && defined(DEBUG) Thread::TaskKind thread_task() const { return ThreadTaskBit::decode(state_); } @@ -368,7 +367,7 @@ class Sample { intptr_t allocation_cid() const { ASSERT(is_allocation_sample()); - return metadata_; + return metadata(); } void set_head_sample(bool head_sample) { @@ -377,25 +376,23 @@ class Sample { bool head_sample() const { return HeadSampleBit::decode(state_); } - void set_metadata(intptr_t metadata) { metadata_ = metadata; } + intptr_t metadata() const { return MetadataBits::decode(state_); } + void set_metadata(intptr_t metadata) { + state_ = MetadataBits::update(metadata, state_); + } void SetAllocationCid(intptr_t cid) { set_is_allocation_sample(true); set_metadata(cid); } - static void Init(); - - static intptr_t instance_size() { return instance_size_; } + static constexpr int kPCArraySizeInWords = 8; + uword* GetPCArray() { return &pc_array_[0]; } - uword* GetPCArray() const; - - static const int kStackBufferSizeInWords = 2; + static constexpr int kStackBufferSizeInWords = 2; uword* GetStackBuffer() { return &stack_buffer_[0]; } private: - static intptr_t instance_size_; - static intptr_t pcs_length_; enum StateBits { kHeadSampleBit = 0, kLeafFrameIsDartBit = 1, @@ -406,42 +403,43 @@ class Sample { kClassAllocationSampleBit = 6, kContinuationSampleBit = 7, kThreadTaskBit = 8, // 6 bits. - kNextFreeBit = 14, - }; - class HeadSampleBit : public BitField {}; - class LeafFrameIsDart : public BitField { + kMetadataBit = 14, // 16 bits. + kNextFreeBit = 30, }; - class IgnoreBit : public BitField {}; - class ExitFrameBit : public BitField {}; + class HeadSampleBit : public BitField {}; + class LeafFrameIsDart + : public BitField {}; + class IgnoreBit : public BitField {}; + class ExitFrameBit : public BitField {}; class MissingFrameInsertedBit - : public BitField {}; + : public BitField {}; class TruncatedTraceBit - : public BitField {}; + : public BitField {}; class ClassAllocationSampleBit - : public BitField {}; + : public BitField {}; class ContinuationSampleBit - : public BitField {}; + : public BitField {}; class ThreadTaskBit - : public BitField {}; + : public BitField {}; + class MetadataBits : public BitField {}; int64_t timestamp_; - ThreadId tid_; Dart_Port port_; - uword pc_marker_; + ThreadId tid_; uword stack_buffer_[kStackBufferSizeInWords]; + uword pc_array_[kPCArraySizeInWords]; uword vm_tag_; uword user_tag_; - uword metadata_; - uword lr_; - uword state_; + uint32_t state_; + int32_t continuation_index_; uint32_t allocation_identity_hash_; + +#if defined(DART_USE_TCMALLOC) && defined(DEBUG) uword native_allocation_address_; uintptr_t native_allocation_size_bytes_; - intptr_t continuation_index_; Sample* next_free_; +#endif - /* There are a variable number of words that follow, the words hold the - * sampled pc values. Access via GetPCArray() */ DISALLOW_COPY_AND_ASSIGN(Sample); }; From ffecb6d44c1fe82a84fd6101fabc9568b20e9e05 Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Thu, 3 Jun 2021 18:28:10 +0000 Subject: [PATCH 10/11] [ package:dds ] Update to package:vm_service ^7.0.0 Change-Id: I3a504f3006fccd1b86356380c07f6641bf1b27ae Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202305 Reviewed-by: Kenzie Schmoll Commit-Queue: Kenzie Schmoll --- pkg/dds/CHANGELOG.md | 3 +++ pkg/dds/pubspec.yaml | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/dds/CHANGELOG.md b/pkg/dds/CHANGELOG.md index c899383d81d8e..84e2e6a14d980 100644 --- a/pkg/dds/CHANGELOG.md +++ b/pkg/dds/CHANGELOG.md @@ -1,3 +1,6 @@ +# 2.0.1 +- Update `package:vm_service` to ^7.0.0. + # 2.0.0 - **Breaking change:** add null safety support. - **Breaking change:** minimum Dart SDK revision bumped to 2.12.0. diff --git a/pkg/dds/pubspec.yaml b/pkg/dds/pubspec.yaml index cf7e0819b8864..2bf3136eea430 100644 --- a/pkg/dds/pubspec.yaml +++ b/pkg/dds/pubspec.yaml @@ -3,7 +3,7 @@ description: >- A library used to spawn the Dart Developer Service, used to communicate with a Dart VM Service instance. -version: 2.0.0 +version: 2.0.1 homepage: https://github.com/dart-lang/sdk/tree/master/pkg/dds @@ -25,7 +25,7 @@ dependencies: sse: ^4.0.0 stream_channel: ^2.0.0 usage: ^4.0.0 - vm_service: ^6.0.1 + vm_service: ^7.0.0 web_socket_channel: ^2.0.0 dev_dependencies: From e1f37f602954681b64ffcd37900f44ef857afd0b Mon Sep 17 00:00:00 2001 From: Keerti Parthasarathy Date: Thu, 3 Jun 2021 20:49:42 +0000 Subject: [PATCH 11/11] Also return lineInfo and oldName from canRename Change-Id: I5ae683cdfe3a82d8dea72745d77d8dcb435c0759 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202280 Commit-Queue: Keerti Parthasarathy Reviewed-by: Konstantin Shcheglov --- pkg/analysis_server/lib/src/cider/rename.dart | 17 ++++++++++-- .../test/src/cider/rename_test.dart | 27 +++++++++---------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/pkg/analysis_server/lib/src/cider/rename.dart b/pkg/analysis_server/lib/src/cider/rename.dart index 72e884b049cc9..6ea1b329173af 100644 --- a/pkg/analysis_server/lib/src/cider/rename.dart +++ b/pkg/analysis_server/lib/src/cider/rename.dart @@ -4,10 +4,19 @@ import 'package:analysis_server/src/services/refactoring/refactoring.dart'; import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/source/line_info.dart'; import 'package:analyzer/src/dart/ast/utilities.dart'; import 'package:analyzer/src/dart/micro/resolve_file.dart'; import 'package:analyzer/src/dart/micro/utils.dart'; +class CanRenameResponse { + final LineInfo lineInfo; + final RenameRefactoringElement refactoringElement; + final String oldName; + + CanRenameResponse(this.lineInfo, this.refactoringElement, this.oldName); +} + class CiderRenameComputer { final FileResolver _fileResolver; @@ -15,7 +24,7 @@ class CiderRenameComputer { /// Check if the identifier at the [line], [column] for the file at the /// [filePath] can be renamed. - RenameRefactoringElement? canRename(String filePath, int line, int column) { + CanRenameResponse? canRename(String filePath, int line, int column) { var resolvedUnit = _fileResolver.resolve(path: filePath); var lineInfo = resolvedUnit.lineInfo; var offset = lineInfo.getOffsetOfLine(line) + column; @@ -35,7 +44,11 @@ class CiderRenameComputer { if (!_canRenameElement(element)) { return null; } - return RenameRefactoring.getElementToRename(node, element); + var refactoring = RenameRefactoring.getElementToRename(node, element); + if (refactoring != null) { + return CanRenameResponse(lineInfo, refactoring, element.displayName); + } + return null; } bool _canRenameElement(Element element) { diff --git a/pkg/analysis_server/test/src/cider/rename_test.dart b/pkg/analysis_server/test/src/cider/rename_test.dart index 290379fbae067..31c96d5a466e4 100644 --- a/pkg/analysis_server/test/src/cider/rename_test.dart +++ b/pkg/analysis_server/test/src/cider/rename_test.dart @@ -3,7 +3,6 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analysis_server/src/cider/rename.dart'; -import 'package:analysis_server/src/services/refactoring/refactoring.dart'; import 'package:analyzer/source/line_info.dart'; import 'package:test/test.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; @@ -31,8 +30,8 @@ class A { '''); expect(refactor, isNotNull); - expect(refactor!.element.name, 'bar'); - expect(refactor.offset, _correctionContext.offset); + expect(refactor!.refactoringElement.element.name, 'bar'); + expect(refactor.refactoringElement.offset, _correctionContext.offset); } void test_canRename_function() { @@ -42,8 +41,8 @@ void ^foo() { '''); expect(refactor, isNotNull); - expect(refactor!.element.name, 'foo'); - expect(refactor.offset, _correctionContext.offset); + expect(refactor!.refactoringElement.element.name, 'foo'); + expect(refactor.refactoringElement.offset, _correctionContext.offset); } void test_canRename_label() { @@ -58,8 +57,8 @@ main() { '''); expect(refactor, isNotNull); - expect(refactor!.element.name, 'myLabel'); - expect(refactor.offset, _correctionContext.offset); + expect(refactor!.refactoringElement.element.name, 'myLabel'); + expect(refactor.refactoringElement.offset, _correctionContext.offset); } void test_canRename_local() { @@ -70,8 +69,8 @@ void foo() { '''); expect(refactor, isNotNull); - expect(refactor!.element.name, 'a'); - expect(refactor.offset, _correctionContext.offset); + expect(refactor!.refactoringElement.element.name, 'a'); + expect(refactor.refactoringElement.offset, _correctionContext.offset); } void test_canRename_method() { @@ -82,8 +81,8 @@ extension E on int { '''); expect(refactor, isNotNull); - expect(refactor!.element.name, 'foo'); - expect(refactor.offset, _correctionContext.offset); + expect(refactor!.refactoringElement.element.name, 'foo'); + expect(refactor.refactoringElement.offset, _correctionContext.offset); } void test_canRename_operator() { @@ -104,11 +103,11 @@ void foo(int ^bar) { '''); expect(refactor, isNotNull); - expect(refactor!.element.name, 'bar'); - expect(refactor.offset, _correctionContext.offset); + expect(refactor!.refactoringElement.element.name, 'bar'); + expect(refactor.refactoringElement.offset, _correctionContext.offset); } - RenameRefactoringElement? _compute(String content) { + CanRenameResponse? _compute(String content) { _updateFile(content); return CiderRenameComputer(