From 78ce85a7fe6f2c0ca80bc6a3073ee9243af11e8a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omersa@google.com>
Date: Thu, 16 Jan 2025 03:24:39 -0800
Subject: [PATCH] [dart2wasm,tfa] Don't infer class of string values in
 comparisons
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In dart2wasm, when a comparison like `x == "hello"` is true, we can't
assume that the class of `x` is the same as the class of `"hello"`:

- If `x` is received from JS, it will be `JSStringImpl`.
- If it's a substring of a `TwoByteString`, it will be `TwoByteString`.
- Otherwise it will be `OneByteString`.

Update `Target` with the new method

```
bool get canInferStringClassAfterEqualityComparison => true;
```

to allow TFA to *not* infer classes of string values after comparisons.

Override the method to return `false` in dart2wasm's `Target`
implementation.

Fixes #59901.
Tested: web/wasm/issue_59901_test
Change-Id: I1a6c8deaf27c54240dd4e821dbd8160914502ad7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404562
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
---
 pkg/dart2wasm/lib/target.dart                  |  7 +++++++
 pkg/kernel/lib/target/targets.dart             |  4 ++++
 .../type_flow/summary_collector.dart           |  1 +
 tests/web/wasm/issue_59901_test.dart           | 18 ++++++++++++++++++
 4 files changed, 30 insertions(+)
 create mode 100644 tests/web/wasm/issue_59901_test.dart

diff --git a/pkg/dart2wasm/lib/target.dart b/pkg/dart2wasm/lib/target.dart
index 87e30c4b7d0..ff37a62a8bf 100644
--- a/pkg/dart2wasm/lib/target.dart
+++ b/pkg/dart2wasm/lib/target.dart
@@ -521,6 +521,13 @@ class WasmTarget extends Target {
         coreTypes.index.getClass('dart:_string', 'OneByteString');
   }
 
+  // In dart2wasm we can't assume that `x == "hello"` means `x`'s class is
+  // `concreteStringLiteralClass("hello")`, it may also be `JSStringImpl` when
+  // it's obtained from a JS call, or `TwoByteString` when it's a substring of a
+  // `TwoByteString`.
+  @override
+  bool get canInferStringClassAfterEqualityComparison => false;
+
   @override
   Class concreteClosureClass(CoreTypes coreTypes) {
     return _closure ??= coreTypes.index.getClass('dart:core', '_Closure');
diff --git a/pkg/kernel/lib/target/targets.dart b/pkg/kernel/lib/target/targets.dart
index fda4156a957..dcdda51e318 100644
--- a/pkg/kernel/lib/target/targets.dart
+++ b/pkg/kernel/lib/target/targets.dart
@@ -508,6 +508,10 @@ abstract class Target {
   Class? concreteDoubleLiteralClass(CoreTypes coreTypes, double value) => null;
   Class? concreteStringLiteralClass(CoreTypes coreTypes, String value) => null;
 
+  /// When a comparison `x == <literal>` is true, whether we can assume the
+  /// class of `x` to be `concreteStringLiteralClass(<literal>)`.
+  bool get canInferStringClassAfterEqualityComparison => true;
+
   Class? concreteAsyncResultClass(CoreTypes coreTypes) => null;
   Class? concreteSyncStarResultClass(CoreTypes coreTypes) => null;
 
diff --git a/pkg/vm/lib/transformations/type_flow/summary_collector.dart b/pkg/vm/lib/transformations/type_flow/summary_collector.dart
index 19bdfccefc5..87cfc9f1afa 100644
--- a/pkg/vm/lib/transformations/type_flow/summary_collector.dart
+++ b/pkg/vm/lib/transformations/type_flow/summary_collector.dart
@@ -1582,6 +1582,7 @@ class SummaryCollector extends RecursiveResultVisitor<TypeExpr?> {
               _isSubtype(lhs.variable.type,
                   _environment.coreTypes.intNullableRawType)) ||
           (rhs is StringLiteral &&
+              target.canInferStringClassAfterEqualityComparison &&
               _isSubtype(lhs.variable.type,
                   _environment.coreTypes.stringNullableRawType)) ||
           (rhs is ConstantExpression &&
diff --git a/tests/web/wasm/issue_59901_test.dart b/tests/web/wasm/issue_59901_test.dart
new file mode 100644
index 00000000000..04b15dfd8ab
--- /dev/null
+++ b/tests/web/wasm/issue_59901_test.dart
@@ -0,0 +1,18 @@
+// Copyright (c) 2025, 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:js_interop';
+
+import 'package:expect/expect.dart';
+
+void main() {
+  final List<int> codeUnits = [];
+  final jsString = "hello".toJS.toDart;
+  if (jsString == "hello") {
+    for (int i = 0; i < 5; i += 1) {
+      codeUnits.add(jsString.codeUnitAt(i));
+    }
+  }
+  Expect.listEquals(codeUnits, [104, 101, 108, 108, 111]);
+}