Skip to content

Commit 5c5ecda

Browse files
johnniwintherCommit Queue
authored andcommitted
[cfe] Avoid overcaching in const conditional simplication
The ConstConditionalSimplifier would try to cache the result of `VariableDeclaration`s but in doing so inadvertently cached parameter values in called extension type constructors. This clears the cache between static invocation, thus avoid the overcaching between functions. Closes #61222 Change-Id: I42f7eb4e0c689d119b88cfd1347baaedf1ff90dd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444000 Reviewed-by: Mayank Patke <[email protected]> Commit-Queue: Johnni Winther <[email protected]> Reviewed-by: Lasse Nielsen <[email protected]> Reviewed-by: Stephen Adams <[email protected]>
1 parent 5ff26a2 commit 5c5ecda

File tree

7 files changed

+261
-8
lines changed

7 files changed

+261
-8
lines changed

pkg/front_end/lib/src/kernel/const_conditional_simplifier.dart

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,12 @@ class ConstConditionalSimplifier extends RemovingTransformer {
7575
super.visitIfStatement(node, removalSentinel);
7676
Constant? condition = _evaluate(node.condition);
7777
if (condition is! BoolConstant) return node;
78-
// Coverage-ignore(suite): Not run.
7978
if (condition.value) {
8079
return node.then;
8180
} else {
82-
return node.otherwise ?? removalSentinel ?? new EmptyStatement();
81+
return node.otherwise ??
82+
removalSentinel ?? // Coverage-ignore(suite): Not run.
83+
new EmptyStatement();
8384
}
8485
}
8586

@@ -121,7 +122,8 @@ class ConstConditionalSimplifier extends RemovingTransformer {
121122
class _ConstantEvaluator extends TryConstantEvaluator {
122123
// TODO(fishythefish): Do caches need to be invalidated when the static type
123124
// context changes?
124-
final Map<VariableDeclaration, Constant?> _variableCache = {};
125+
/// Cache for local variables in the current method.
126+
Map<VariableDeclaration, Constant?> _variableCache = {};
125127
final Map<Field, Constant?> _staticFieldCache = {};
126128
final Map<FunctionNode, Constant?> _functionCache = {};
127129
final Map<FunctionNode, Constant?> _localFunctionCache = {};
@@ -143,17 +145,14 @@ class _ConstantEvaluator extends TryConstantEvaluator {
143145

144146
Constant? _evaluateFunctionInvocation(FunctionNode node) {
145147
if (node.typeParameters.isNotEmpty ||
146-
// Coverage-ignore(suite): Not run.
147148
node.requiredParameterCount != 0 ||
148-
// Coverage-ignore(suite): Not run.
149149
node.positionalParameters.isNotEmpty ||
150-
// Coverage-ignore(suite): Not run.
151150
node.namedParameters.isNotEmpty) {
152151
return null;
153152
}
154-
// Coverage-ignore-block(suite): Not run.
155153
Statement? body = node.body;
156154
if (body is! ReturnStatement) return null;
155+
// Coverage-ignore-block(suite): Not run.
157156
Expression? expression = body.expression;
158157
if (expression == null) return null;
159158
return _evaluate(expression);
@@ -236,7 +235,16 @@ class _ConstantEvaluator extends TryConstantEvaluator {
236235

237236
@override
238237
Constant visitStaticInvocation(StaticInvocation node) {
239-
return _lookupStaticInvocation(node.target) ??
238+
// Clear variable cache static invocations to avoid looking up cached
239+
// variables from another call stack.
240+
//
241+
// This can occur when calling const extension type constructors since these
242+
// are lowered into top level functions.
243+
Map<VariableDeclaration, Constant?> oldCache = _variableCache;
244+
_variableCache = {};
245+
Constant result = _lookupStaticInvocation(node.target) ??
240246
super.visitStaticInvocation(node);
247+
_variableCache = oldCache;
248+
return result;
241249
}
242250
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright (c) 2025, 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+
void main() {
6+
const [E.vn(), E.v1()];
7+
var eList = <E>[?E.vn(), ?E.v1()];
8+
9+
const [C.vn(), C.v1()];
10+
var cList = <C>[if (C.vn() != C.v1()) C.v1()];
11+
}
12+
13+
extension type const E._(int? _) {
14+
const E.vn() : this._(null);
15+
const E.v1() : this._(1);
16+
}
17+
18+
class C {
19+
final int? field;
20+
const C._(this.field);
21+
const C.vn() : this._(null);
22+
const C.v1() : this._(1);
23+
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
library;
2+
import self as self;
3+
import "dart:core" as core;
4+
5+
class C extends core::Object /*hasConstConstructor*/ {
6+
final field core::int? field;
7+
const constructor _(core::int? field) → self::C
8+
: self::C::field = field, super core::Object::•()
9+
;
10+
const constructor vn() → self::C
11+
: this self::C::_(null)
12+
;
13+
const constructor v1() → self::C
14+
: this self::C::_(1)
15+
;
16+
static synthetic method _#_#tearOff(core::int? field) → self::C
17+
return new self::C::_(field);
18+
static synthetic method _#vn#tearOff() → self::C
19+
return new self::C::vn();
20+
static synthetic method _#v1#tearOff() → self::C
21+
return new self::C::v1();
22+
}
23+
extension type E(core::int? _) {
24+
abstract extension-type-member representation-field get _() → core::int?;
25+
constructor _ = self::E|constructor#_;
26+
constructor tearoff _ = self::E|constructor#_#_#tearOff;
27+
constructor vn = self::E|constructor#vn;
28+
constructor tearoff vn = self::E|constructor#_#vn#tearOff;
29+
constructor v1 = self::E|constructor#v1;
30+
constructor tearoff v1 = self::E|constructor#_#v1#tearOff;
31+
}
32+
static method main() → void {
33+
#C3;
34+
core::List<self::E% /* erasure=core::int?, declared=! */> eList = block {
35+
final core::List<self::E% /* erasure=core::int?, declared=! */> #t1 = <self::E% /* erasure=core::int?, declared=! */>[];
36+
final self::E? /* erasure=core::int? */ #t2 = self::E|constructor#vn();
37+
if(!(#t2 == null))
38+
#t1.{core::List::add}{Invariant}(#t2{self::E% /* erasure=core::int?, declared=! */}){(self::E% /* erasure=core::int?, declared=! */) → void};
39+
final self::E? /* erasure=core::int? */ #t3 = self::E|constructor#v1();
40+
if(!(#t3 == null))
41+
#t1.{core::List::add}{Invariant}(#t3{self::E% /* erasure=core::int?, declared=! */}){(self::E% /* erasure=core::int?, declared=! */) → void};
42+
} =>#t1;
43+
#C6;
44+
core::List<self::C> cList = block {
45+
final core::List<self::C> #t4 = <self::C>[];
46+
if(!(new self::C::vn() =={core::Object::==}{(core::Object) → core::bool} new self::C::v1()))
47+
#t4.{core::List::add}{Invariant}(new self::C::v1()){(self::C) → void};
48+
} =>#t4;
49+
}
50+
static extension-type-member method E|constructor#_(core::int? _) → self::E% /* erasure=core::int?, declared=! */ {
51+
lowered final self::E% /* erasure=core::int?, declared=! */ #this = _;
52+
return #this;
53+
}
54+
static extension-type-member synthetic method E|constructor#_#_#tearOff(core::int? _) → self::E% /* erasure=core::int?, declared=! */
55+
return self::E|constructor#_(_);
56+
static extension-type-member method E|constructor#vn() → self::E% /* erasure=core::int?, declared=! */ {
57+
lowered final self::E% /* erasure=core::int?, declared=! */ #this;
58+
#this = self::E|constructor#_(null);
59+
return #this;
60+
}
61+
static extension-type-member synthetic method E|constructor#_#vn#tearOff() → self::E% /* erasure=core::int?, declared=! */
62+
return self::E|constructor#vn();
63+
static extension-type-member method E|constructor#v1() → self::E% /* erasure=core::int?, declared=! */ {
64+
lowered final self::E% /* erasure=core::int?, declared=! */ #this;
65+
#this = self::E|constructor#_(1);
66+
return #this;
67+
}
68+
static extension-type-member synthetic method E|constructor#_#v1#tearOff() → self::E% /* erasure=core::int?, declared=! */
69+
return self::E|constructor#v1();
70+
71+
constants {
72+
#C1 = null
73+
#C2 = 1
74+
#C3 = <core::int?>[#C1, #C2]
75+
#C4 = self::C {field:#C1}
76+
#C5 = self::C {field:#C2}
77+
#C6 = <self::C>[#C4, #C5]
78+
}
79+
80+
81+
Constructor coverage from constants:
82+
org-dartlang-testcase:///issue61222.dart:
83+
- C.vn (from org-dartlang-testcase:///issue61222.dart:21:9)
84+
- C._ (from org-dartlang-testcase:///issue61222.dart:20:9)
85+
- Object. (from org-dartlang-sdk:///lib/core/object.dart)
86+
- C.v1 (from org-dartlang-testcase:///issue61222.dart:22:9)
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
library;
2+
import self as self;
3+
import "dart:core" as core;
4+
5+
class C extends core::Object /*hasConstConstructor*/ {
6+
final field core::int? field;
7+
const constructor _(final core::int? field) → self::C
8+
: self::C::field = field, super core::Object::•()
9+
;
10+
const constructor vn() → self::C
11+
: this self::C::_(null)
12+
;
13+
const constructor v1() → self::C
14+
: this self::C::_(1)
15+
;
16+
static synthetic method _#_#tearOff(final core::int? field) → self::C
17+
return new self::C::_(field);
18+
static synthetic method _#vn#tearOff() → self::C
19+
return new self::C::vn();
20+
static synthetic method _#v1#tearOff() → self::C
21+
return new self::C::v1();
22+
}
23+
extension type E(core::int? _) {
24+
abstract extension-type-member representation-field get _() → core::int?;
25+
constructor _ = self::E|constructor#_;
26+
constructor tearoff _ = self::E|constructor#_#_#tearOff;
27+
constructor vn = self::E|constructor#vn;
28+
constructor tearoff vn = self::E|constructor#_#vn#tearOff;
29+
constructor v1 = self::E|constructor#v1;
30+
constructor tearoff v1 = self::E|constructor#_#v1#tearOff;
31+
}
32+
static method main() → void {
33+
#C3;
34+
final core::List<self::E% /* erasure=core::int?, declared=! */> eList = block {
35+
final core::List<self::E% /* erasure=core::int?, declared=! */> #t1 = <self::E% /* erasure=core::int?, declared=! */>[];
36+
final self::E? /* erasure=core::int? */ #t2 = self::E|constructor#vn();
37+
final self::E? /* erasure=core::int? */ #t3 = self::E|constructor#v1();
38+
#t1.{core::List::add}{Invariant}(#t3{self::E% /* erasure=core::int?, declared=! */}){(self::E% /* erasure=core::int?, declared=! */) → void};
39+
} =>#t1;
40+
#C6;
41+
final core::List<self::C> cList = block {
42+
final core::List<self::C> #t4 = <self::C>[];
43+
if(!(new self::C::vn() =={core::Object::==}{(core::Object) → core::bool} new self::C::v1()))
44+
#t4.{core::List::add}{Invariant}(new self::C::v1()){(self::C) → void};
45+
} =>#t4;
46+
}
47+
static extension-type-member method E|constructor#_(final core::int? _) → self::E% /* erasure=core::int?, declared=! */ {
48+
lowered final self::E% /* erasure=core::int?, declared=! */ #this = _;
49+
return #this;
50+
}
51+
static extension-type-member synthetic method E|constructor#_#_#tearOff(final core::int? _) → self::E% /* erasure=core::int?, declared=! */
52+
return self::E|constructor#_(_);
53+
static extension-type-member method E|constructor#vn() → self::E% /* erasure=core::int?, declared=! */ {
54+
lowered final self::E% /* erasure=core::int?, declared=! */ #this;
55+
#this = self::E|constructor#_(null);
56+
return #this;
57+
}
58+
static extension-type-member synthetic method E|constructor#_#vn#tearOff() → self::E% /* erasure=core::int?, declared=! */
59+
return self::E|constructor#vn();
60+
static extension-type-member method E|constructor#v1() → self::E% /* erasure=core::int?, declared=! */ {
61+
lowered final self::E% /* erasure=core::int?, declared=! */ #this;
62+
#this = self::E|constructor#_(1);
63+
return #this;
64+
}
65+
static extension-type-member synthetic method E|constructor#_#v1#tearOff() → self::E% /* erasure=core::int?, declared=! */
66+
return self::E|constructor#v1();
67+
68+
constants {
69+
#C1 = null
70+
#C2 = 1
71+
#C3 = <core::int?>[#C1, #C2]
72+
#C4 = self::C {field:#C1}
73+
#C5 = self::C {field:#C2}
74+
#C6 = <self::C>[#C4, #C5]
75+
}
76+
77+
Extra constant evaluation status:
78+
Evaluated: FactoryConstructorInvocation @ org-dartlang-testcase:///issue61222.dart:7:22 -> NullConstant(null)
79+
Evaluated: FactoryConstructorInvocation @ org-dartlang-testcase:///issue61222.dart:7:31 -> IntConstant(1)
80+
Evaluated: StaticInvocation @ org-dartlang-testcase:///issue61222.dart:14:23 -> NullConstant(null)
81+
Evaluated: StaticInvocation @ org-dartlang-testcase:///issue61222.dart:14:9 -> NullConstant(null)
82+
Evaluated: StaticInvocation @ org-dartlang-testcase:///issue61222.dart:15:23 -> IntConstant(1)
83+
Evaluated: StaticInvocation @ org-dartlang-testcase:///issue61222.dart:15:9 -> IntConstant(1)
84+
Extra constant evaluation: evaluated: 35, effectively constant: 6
85+
86+
87+
Constructor coverage from constants:
88+
org-dartlang-testcase:///issue61222.dart:
89+
- C.vn (from org-dartlang-testcase:///issue61222.dart:21:9)
90+
- C._ (from org-dartlang-testcase:///issue61222.dart:20:9)
91+
- Object. (from org-dartlang-sdk:///lib/core/object.dart)
92+
- C.v1 (from org-dartlang-testcase:///issue61222.dart:22:9)
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
void main() {}
2+
3+
extension type const E._(int? _) {
4+
const E.vn() : this._(null);
5+
const E.v1() : this._(1);
6+
}
7+
8+
class C {
9+
final int? field;
10+
const C._(this.field);
11+
const C.vn() : this._(null);
12+
const C.v1() : this._(1);
13+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
class C {
2+
const C._(this.field);
3+
const C.v1() : this._(1);
4+
const C.vn() : this._(null);
5+
final int? field;
6+
}
7+
8+
extension type const E._(int? _) {
9+
const E.v1() : this._(1);
10+
const E.vn() : this._(null);
11+
}
12+
13+
void main() {}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright (c) 2025, 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+
// This tests that extension constructor calls are not wrongfully optimized
6+
// away during const conditional simplication.
7+
8+
import 'package:expect/expect.dart';
9+
10+
void main() {
11+
var eList = <E>[?E.vn(), ?E.v1()];
12+
Expect.isTrue(eList.isNotEmpty);
13+
}
14+
15+
extension type const E._(int? _) {
16+
const E.vn() : this._(null);
17+
const E.v1() : this._(1);
18+
}

0 commit comments

Comments
 (0)