diff --git a/pkg/vm/lib/transformations/type_flow/protobuf_handler.dart b/pkg/vm/lib/transformations/type_flow/protobuf_handler.dart index 865f56d5902..c1f616ddd0a 100644 --- a/pkg/vm/lib/transformations/type_flow/protobuf_handler.dart +++ b/pkg/vm/lib/transformations/type_flow/protobuf_handler.dart @@ -68,6 +68,8 @@ class ProtobufHandler { _builderInfoAddMethod = libraryIndex.getMember(protobufLibraryUri, 'BuilderInfo', 'add'); + bool usesAnnotationClass(Class cls) => cls == _tagNumberClass; + /// This method is called from summary collector when analysis discovered /// that [member] is called and needs to construct a summary for its body. /// diff --git a/pkg/vm/lib/transformations/type_flow/summary_collector.dart b/pkg/vm/lib/transformations/type_flow/summary_collector.dart index a20c8405993..9e73f760dcd 100644 --- a/pkg/vm/lib/transformations/type_flow/summary_collector.dart +++ b/pkg/vm/lib/transformations/type_flow/summary_collector.dart @@ -774,6 +774,12 @@ class SummaryCollector extends RecursiveResultVisitor { _summary.result = _returnValue; } + // Visit annotations on members, classes and libraries. + // Other nodes currently do not have annotations used by the VM. + member.annotations.forEach(_visit); + member.enclosingClass?.annotations?.forEach(_visit); + member.enclosingLibrary?.annotations?.forEach(_visit); + _staticTypeContext = null; debugPrint("------------ SUMMARY ------------"); diff --git a/pkg/vm/lib/transformations/type_flow/transformer.dart b/pkg/vm/lib/transformations/type_flow/transformer.dart index e7ac6527a6c..5dcdfe7f74d 100644 --- a/pkg/vm/lib/transformations/type_flow/transformer.dart +++ b/pkg/vm/lib/transformations/type_flow/transformer.dart @@ -75,6 +75,9 @@ Component transformComponent( typeFlowAnalysis.addRawCall(mainSelector); } + CleanupAnnotations(coreTypes, libraryIndex, protobufHandler) + .visitComponent(component); + typeFlowAnalysis.process(); analysisStopWatch.stop(); @@ -118,6 +121,56 @@ Component transformComponent( return component; } +// Pass which removes all annotations except @ExternalName and @pragma +// on members, classes and libraries. May also keep @TagNumber which is used +// by protobuf handler. +class CleanupAnnotations extends RecursiveVisitor { + final Class externalNameClass; + final Class pragmaClass; + final ProtobufHandler protobufHandler; + + CleanupAnnotations( + CoreTypes coreTypes, LibraryIndex index, this.protobufHandler) + : externalNameClass = index.getClass('dart:_internal', 'ExternalName'), + pragmaClass = coreTypes.pragmaClass; + + @override + defaultNode(Node node) { + if (node is Annotatable && node.annotations.isNotEmpty) { + _cleanupAnnotations(node, node.annotations); + } + super.defaultNode(node); + } + + @override + visitTypedef(Typedef node) { + super.visitTypedef(node); + // These sub-nodes can have annotations but are not visited by + // Typedef.visitChildren. + visitList(node.positionalParameters, this); + visitList(node.namedParameters, this); + } + + void _cleanupAnnotations(Node node, List annotations) { + if (node is Member || node is Class || node is Library) { + annotations.removeWhere((a) => !_keepAnnotation(a)); + } else { + annotations.clear(); + } + } + + bool _keepAnnotation(Expression annotation) { + final constant = (annotation as ConstantExpression).constant; + if (constant is InstanceConstant) { + final cls = constant.classNode; + return (cls == externalNameClass) || + (cls == pragmaClass) || + (protobufHandler != null && protobufHandler.usesAnnotationClass(cls)); + } + return false; + } +} + /// Devirtualization based on results of type flow analysis. class TFADevirtualization extends Devirtualization { final TypeFlowAnalysis _typeFlowAnalysis; @@ -731,6 +784,7 @@ class TreeShaker { void addUsedExtension(Extension node) { if (_usedExtensions.add(node)) { + node.annotations = const []; _pass1.transformTypeParameterList(node.typeParameters, node); node.onType?.accept(typeVisitor); } @@ -738,7 +792,7 @@ class TreeShaker { void addUsedTypedef(Typedef typedef) { if (_usedTypedefs.add(typedef)) { - _pass1.transformExpressionList(typedef.annotations, typedef); + typedef.annotations = const []; _pass1.transformTypeParameterList(typedef.typeParameters, typedef); _pass1.transformTypeParameterList( typedef.typeParametersOfFunctionType, typedef); @@ -1719,7 +1773,10 @@ class _TreeShakerConstantVisitor extends ConstantVisitor { shaker.addClassUsedInType(constant.classNode); visitList(constant.typeArguments, typeVisitor); constant.fieldValues.forEach((Reference fieldRef, Constant value) { - shaker.addUsedMember(fieldRef.asField); + if (!shaker.retainField(fieldRef.asField)) { + throw 'Constant $constant references field ${fieldRef.asField} ' + 'which is not retained'; + } analyzeConstant(value); }); } diff --git a/pkg/vm/testcases/transformations/type_flow/transformer/annotation.dart.expect b/pkg/vm/testcases/transformations/type_flow/transformer/annotation.dart.expect index 28378bee51e..a1350daa1fd 100644 --- a/pkg/vm/testcases/transformations/type_flow/transformer/annotation.dart.expect +++ b/pkg/vm/testcases/transformations/type_flow/transformer/annotation.dart.expect @@ -2,37 +2,20 @@ library #lib; import self as self; import "dart:core" as core; -@#C5 typedef SomeType = (core::List*) →* void; -abstract class ClassAnnotation2 extends core::Object /*hasConstConstructor*/ { -} -abstract class MethodAnnotation extends core::Object /*hasConstConstructor*/ { -[@vm.unreachable.metadata=] [@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:1] final field core::int* x; -} -abstract class TypedefAnnotation extends core::Object /*hasConstConstructor*/ { -[@vm.unreachable.metadata=] [@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:2] final field core::List* list; -} -abstract class VarAnnotation extends core::Object /*hasConstConstructor*/ { -} -abstract class ParametrizedAnnotation extends core::Object /*hasConstConstructor*/ { -[@vm.unreachable.metadata=] [@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:3] final field self::ParametrizedAnnotation::T* foo; -} abstract class A extends core::Object { static method staticMethod() → void {} } -@#C6 class B extends core::Object { synthetic constructor •() → self::B* : super core::Object::•() ; -[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:4,getterSelectorId:5] @#C8 - method instanceMethod() → void {} +[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:1,getterSelectorId:2] method instanceMethod() → void {} } [@vm.unboxing-info.metadata=()->i]static method foo() → core::int* { - @#C9 core::int* x = 2; + core::int* x = 2; return [@vm.direct-call.metadata=dart.core::_IntegerImplementation.+] [@vm.inferred-type.metadata=int (skip check)] x.{core::num::+}(2); } -@#C11 static method main(core::List* args) → dynamic { self::A::staticMethod(); [@vm.direct-call.metadata=#lib::B.instanceMethod] [@vm.inferred-type.metadata=!? (skip check)] new self::B::•().{self::B::instanceMethod}(); diff --git a/runtime/tests/vm/dart/regress_45968_test.dart b/runtime/tests/vm/dart/regress_45968_test.dart new file mode 100644 index 00000000000..80cff1fb7c9 --- /dev/null +++ b/runtime/tests/vm/dart/regress_45968_test.dart @@ -0,0 +1,28 @@ +// Copyright (c) 2021, 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. + +// Regression test for https://github.com/dart-lang/sdk/issues/45968. +// Verifies that compiler doesn't crash if annotation references field +// which is replaced with a getter. + +class Qualifier { + final String name; + const Qualifier(this.name); +} + +class Foo implements Qualifier { + String get name => 'a'; +} + +class Bar { + @Qualifier('b') + void bar() {} +} + +Qualifier x = Foo(); + +main() { + print(x.name); + Bar().bar(); +} diff --git a/runtime/tests/vm/dart_2/regress_45968_test.dart b/runtime/tests/vm/dart_2/regress_45968_test.dart new file mode 100644 index 00000000000..80cff1fb7c9 --- /dev/null +++ b/runtime/tests/vm/dart_2/regress_45968_test.dart @@ -0,0 +1,28 @@ +// Copyright (c) 2021, 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. + +// Regression test for https://github.com/dart-lang/sdk/issues/45968. +// Verifies that compiler doesn't crash if annotation references field +// which is replaced with a getter. + +class Qualifier { + final String name; + const Qualifier(this.name); +} + +class Foo implements Qualifier { + String get name => 'a'; +} + +class Bar { + @Qualifier('b') + void bar() {} +} + +Qualifier x = Foo(); + +main() { + print(x.name); + Bar().bar(); +}