Skip to content

Commit c9b28eb

Browse files
alexmarkovCommit Queue
authored and
Commit Queue
committed
[vm/aot/tfa] Handle references to tree-shaken closures
Sometimes inferred closure value may reference a closure which was eliminated by the tree shaker (as tree-shaker can prune deeper AST). That should not cause a crash and inferred closure value should be disregarded. TEST=pkg/vm/testcases/transformations/type_flow/transformer/eliminated_closure.dart Fixes b/305730721 Change-Id: Ia26e620ebc06f949be1e05c734be65ce1d708b64 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330661 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
1 parent c8a782f commit c9b28eb

File tree

4 files changed

+81
-3
lines changed

4 files changed

+81
-3
lines changed

pkg/vm/lib/metadata/closure_id.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,12 @@ class ClosureIdMetadataRepository extends MetadataRepository<int> {
2828
return source.readUInt30();
2929
}
3030

31-
/// Return closure ID within the enclosing [Member].
31+
/// Return closure ID within the enclosing [Member], or -1
32+
/// if closure was not indexed.
3233
///
3334
/// Closures should be indexed within enclosing [Member]
3435
/// using [indexClosures].
35-
int getClosureId(LocalFunction closure) => mapping[closure]!;
36+
int getClosureId(LocalFunction closure) => mapping[closure] ?? -1;
3637

3738
/// Assign IDs for all closures within [member].
3839
void indexClosures(Member member) {

pkg/vm/lib/transformations/type_flow/transformer.dart

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,14 @@ class AnnotateKernel extends RecursiveVisitor {
378378
if (function != null) {
379379
_closureIdMetadata.indexClosures(closureMember);
380380
closureId = _closureIdMetadata.getClosureId(function);
381-
assert(closureId > 0);
381+
if (closureId < 0) {
382+
// Closure was tree-shaken and doesn't belong to
383+
// the body of [closureMember].
384+
closureMember = null;
385+
closureId = 0;
386+
} else {
387+
assert(closureId > 0);
388+
}
382389
} else {
383390
closureId = 0;
384391
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
abstract class A {
6+
List<D> get bar;
7+
}
8+
9+
class B implements A {
10+
List<D> bar;
11+
B(this.bar);
12+
}
13+
14+
abstract class C {
15+
A get foo;
16+
17+
String toString() => foo.bar.map((key) => key.baz((arg) {})).join('\n');
18+
}
19+
20+
class D {
21+
String baz(void Function(dynamic) callback) {
22+
print(callback);
23+
return 'hey';
24+
}
25+
}
26+
27+
class E extends C {
28+
A get foo => throw 'Not today';
29+
}
30+
31+
void main() {
32+
print(D());
33+
print(E());
34+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
library #lib;
2+
import self as self;
3+
import "dart:core" as core;
4+
5+
abstract class A extends core::Object {
6+
}
7+
abstract class C extends core::Object {
8+
synthetic constructor •() → self::C
9+
: super core::Object::•()
10+
;
11+
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:1] abstract get foo() → self::A;
12+
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:2,getterSelectorId:3] [@vm.closure-id=0] method toString() → core::String
13+
return block {
14+
[@vm.direct-call.metadata=#lib::E.foo] [@vm.inferred-type.metadata=!] this.{self::C::foo}{self::A};
15+
} =>throw "Attempt to execute code removed by Dart AOT compiler (TFA)";
16+
}
17+
class D extends core::Object {
18+
synthetic constructor •() → self::D
19+
: super core::Object::•()
20+
;
21+
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:4,getterSelectorId:5] method baz([@vm.inferred-arg-type.metadata=dart.core::_Closure] (dynamic) → void callback) → core::String {
22+
core::print(callback);
23+
return "hey";
24+
}
25+
}
26+
class E extends self::C {
27+
synthetic constructor •() → self::E
28+
: super self::C::•()
29+
;
30+
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:1] get foo() → self::A
31+
return throw "Not today";
32+
}
33+
static method main() → void {
34+
core::print(new self::D::•());
35+
core::print(new self::E::•());
36+
}

0 commit comments

Comments
 (0)