[dart2wasm] Fix remaining issues around [Symbol] and privacy

mkustermann · Commit Queue · commit 8b495a04bd98 · 2025-06-17T13:07:20.000-07:00
Symbol comparison & hashing has to take library privacy into account. We refactor the compiler to move symbol related logic to a new `pkg/dart2wasm/lib/symbols.dart:Symbols`. It will be responsible for making symbol constants, mangling and minification. In there we encapsulate the name mangling required to make library privacy work (similar to how the VM does it). We also clean up the runtime implementation (which was a copy of the VM) and make the unmangling be based purely on removing trailing `...@<number>`. Fixes the following tests * co19/LibTest/core/Symbol/Symbol_A03_t01 * co19/LibTest/core/Symbol/Symbol_A03_t02 * language/private/private_test Closes #50791 Change-Id: If1bb19f3d8868662531162a0458754007017df33 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/435140 Commit-Queue: Martin Kustermann <kustermann@google.com> Reviewed-by: Ömer Ağacan <omersa@google.com>
diff --git a/pkg/dart2wasm/lib/code_generator.dart b/pkg/dart2wasm/lib/code_generator.dart
@@ -1741,7 +1741,7 @@ abstract class AstCodeGenerator
     final typeArguments = node.arguments.types;
     final positionalArguments = node.arguments.positional;
     final namedArguments = node.arguments.named;
-    final memberName = node.name.text;
+    final memberName = node.name;
     final forwarder = translator
         .getDynamicForwardersForModule(b.module)
         .getDynamicInvocationForwarder(memberName);
@@ -1788,8 +1788,8 @@ abstract class AstCodeGenerator
         final name = namedArgumentLocals[elementIdx ~/ 2].key;
         final w.ValueType symbolValueType =
             translator.classInfo[translator.symbolClass]!.nonNullableType;
-        translator.constants.instantiateConstant(
-            b, SymbolConstant(name, null), symbolValueType);
+        translator.constants.instantiateConstant(b,
+            translator.symbols.symbolForNamedParameter(name), symbolValueType);
       } else {
         final local = namedArgumentLocals[elementIdx ~/ 2].value;
         b.local_get(local);
@@ -2165,7 +2165,7 @@ abstract class AstCodeGenerator
   @override
   w.ValueType visitDynamicGet(DynamicGet node, w.ValueType expectedType) {
     final receiver = node.receiver;
-    final memberName = node.name.text;
+    final memberName = node.name;
     final forwarder = translator
         .getDynamicForwardersForModule(b.module)
         .getDynamicGetForwarder(memberName);
@@ -2198,7 +2198,7 @@ abstract class AstCodeGenerator
   w.ValueType visitDynamicSet(DynamicSet node, w.ValueType expectedType) {
     final receiver = node.receiver;
     final value = node.value;
-    final memberName = node.name.text;
+    final memberName = node.name;
     final forwarder = translator
         .getDynamicForwardersForModule(b.module)
         .getDynamicSetForwarder(memberName);
@@ -3179,7 +3179,7 @@ abstract class AstCodeGenerator
     }
     translator.constants.instantiateConstant(
         b,
-        SymbolConstant(member.name.text, null),
+        translator.symbols.methodSymbolFromName(member.name),
         translator.classInfo[translator.symbolClass]!.nonNullableType);
     call(translator
         .noSuchMethodErrorThrowUnimplementedExternalMemberError.reference);
diff --git a/pkg/dart2wasm/lib/constants.dart b/pkg/dart2wasm/lib/constants.dart
@@ -2,7 +2,6 @@
 // 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:convert';
 import 'dart:typed_data';
 
 import 'package:kernel/ast.dart';
@@ -80,18 +79,6 @@ class Constants {
   w.DataSegmentBuilder? int32Segment;
   late final ClassInfo typeInfo = translator.classInfo[translator.typeClass]!;
 
-  final Map<String, int> symbolOrdinals = {};
-
-  String minifySymbol(String originalValue) {
-    if (translator.options.minify) {
-      int symbolOrdinal = symbolOrdinals.putIfAbsent(
-          originalValue, () => symbolOrdinals.length);
-      return _intToBase64(symbolOrdinal);
-    } else {
-      return originalValue;
-    }
-  }
-
   final Map<DartType, InstanceConstant> _loweredTypeConstants = {};
   late final BoolConstant _cachedTrueConstant = BoolConstant(true);
   late final BoolConstant _cachedFalseConstant = BoolConstant(false);
@@ -145,15 +132,17 @@ class Constants {
   }
 
   /// Makes a `_NamedParameter` [InstanceConstant].
-  InstanceConstant makeNamedParameterConstant(NamedType n) =>
-      InstanceConstant(translator.namedParameterClass.reference, const [], {
-        translator.namedParameterNameField.fieldReference:
-            SymbolConstant(n.name, null),
-        translator.namedParameterTypeField.fieldReference:
-            _lowerTypeConstant(n.type),
-        translator.namedParameterIsRequiredField.fieldReference:
-            BoolConstant(n.isRequired),
-      });
+  InstanceConstant makeNamedParameterConstant(NamedType n) {
+    return InstanceConstant(
+        translator.namedParameterClass.reference, const [], {
+      translator.namedParameterNameField.fieldReference:
+          translator.symbols.symbolForNamedParameter(n.name),
+      translator.namedParameterTypeField.fieldReference:
+          _lowerTypeConstant(n.type),
+      translator.namedParameterIsRequiredField.fieldReference:
+          BoolConstant(n.isRequired),
+    });
+  }
 
   /// Creates a `WasmArray<_NamedParameter>` to be used as field of
   /// `_FunctionType`.
@@ -1176,8 +1165,8 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
     ClassInfo info = translator.classInfo[translator.symbolClass]!;
     translator.functions.recordClassAllocation(info.classId);
     w.RefType stringType = translator.stringType;
-    final String symbolStringValue = constants.minifySymbol(constant.name);
-    StringConstant nameConstant = StringConstant(symbolStringValue);
+    final nameConstant =
+        StringConstant(translator.symbols.getMangledSymbolName(constant));
     bool lazy = ensureConstant(nameConstant)?.isLazy ?? false;
     return createConstant(constant, info.nonNullableType, lazy: lazy, (b) {
       b.pushObjectHeaderFields(translator, info);
@@ -1211,19 +1200,6 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
   }
 }
 
-List<int> _intToLittleEndianBytes(int i) {
-  List<int> bytes = [];
-  bytes.add(i & 0xFF);
-  i >>>= 8;
-  while (i != 0) {
-    bytes.add(i & 0xFF);
-    i >>>= 8;
-  }
-  return bytes;
-}
-
-String _intToBase64(int i) => base64.encode(_intToLittleEndianBytes(i));
-
 /// Resolves to true if the visited Constant is accessible from dynamic
 /// submodules.
 ///
diff --git a/pkg/dart2wasm/lib/dispatch_table.dart b/pkg/dart2wasm/lib/dispatch_table.dart
@@ -400,13 +400,13 @@ class DispatchTable {
   final Map<int, SelectorInfo> _selectorInfo = {};
 
   /// Maps member names to getter selectors with the same member name.
-  final Map<String, Set<SelectorInfo>> _dynamicGetters = {};
+  final Map<Name, Set<SelectorInfo>> _dynamicGetters = {};
 
   /// Maps member names to setter selectors with the same member name.
-  final Map<String, Set<SelectorInfo>> _dynamicSetters = {};
+  final Map<Name, Set<SelectorInfo>> _dynamicSetters = {};
 
   /// Maps member names to method selectors with the same member name.
-  final Map<String, Set<SelectorInfo>> _dynamicMethods = {};
+  final Map<Name, Set<SelectorInfo>> _dynamicMethods = {};
 
   /// Contents of [_definedWasmTable]. For a selector with ID S and a target
   /// class of the selector with ID C, `table[S + C]` gives the reference to the
@@ -427,7 +427,7 @@ class DispatchTable {
     sink.writeList(_selectorInfo.values, (s) => s.serialize(sink));
     sink.writeList(_table, (r) => sink.writeNullable(r, sink.writeReference));
     // Preserve call selectors for closure calls which are handled dynamically.
-    final callSelectors = _dynamicGetters['call']!;
+    final callSelectors = _dynamicGetters[Name('call')]!;
     sink.writeList(callSelectors, (s) => sink.writeInt(s.id));
   }
 
@@ -450,7 +450,7 @@ class DispatchTable {
     for (final selectorId in callSelectorIds) {
       callSelectors.add(dispatchTable._selectorInfo[selectorId]!);
     }
-    dispatchTable._dynamicGetters['call'] = callSelectors;
+    dispatchTable._dynamicGetters[Name('call')] = callSelectors;
 
     return dispatchTable;
   }
@@ -497,27 +497,27 @@ class DispatchTable {
 
     if (calledDynamically) {
       if (isGetter) {
-        (_dynamicGetters[member.name.text] ??= {}).add(selector);
+        (_dynamicGetters[member.name] ??= {}).add(selector);
       } else if (isSetter) {
-        (_dynamicSetters[member.name.text] ??= {}).add(selector);
+        (_dynamicSetters[member.name] ??= {}).add(selector);
       } else {
-        (_dynamicMethods[member.name.text] ??= {}).add(selector);
+        (_dynamicMethods[member.name] ??= {}).add(selector);
       }
     }
 
     return selector;
   }
 
   /// Get selectors for getters and tear-offs with the given name.
-  Iterable<SelectorInfo> dynamicGetterSelectors(String memberName) =>
+  Iterable<SelectorInfo> dynamicGetterSelectors(Name memberName) =>
       _dynamicGetters[memberName] ?? Iterable.empty();
 
   /// Get selectors for setters with the given name.
-  Iterable<SelectorInfo> dynamicSetterSelectors(String memberName) =>
+  Iterable<SelectorInfo> dynamicSetterSelectors(Name memberName) =>
       _dynamicSetters[memberName] ?? Iterable.empty();
 
   /// Get selectors for methods with the given name.
-  Iterable<SelectorInfo> dynamicMethodSelectors(String memberName) =>
+  Iterable<SelectorInfo> dynamicMethodSelectors(Name memberName) =>
       _dynamicMethods[memberName] ?? Iterable.empty();
 
   void _initializeWasmTable() {
diff --git a/pkg/dart2wasm/lib/dynamic_forwarders.dart b/pkg/dart2wasm/lib/dynamic_forwarders.dart
@@ -18,21 +18,21 @@ class DynamicForwarders {
   final Translator translator;
   final w.ModuleBuilder callingModule;
 
-  final Map<String, CallTarget> _getterForwarderOfName = {};
-  final Map<String, CallTarget> _setterForwarderOfName = {};
-  final Map<String, CallTarget> _methodForwarderOfName = {};
+  final Map<Name, CallTarget> _getterForwarderOfName = {};
+  final Map<Name, CallTarget> _setterForwarderOfName = {};
+  final Map<Name, CallTarget> _methodForwarderOfName = {};
 
   DynamicForwarders(this.translator, this.callingModule);
 
-  CallTarget getDynamicGetForwarder(String memberName) =>
+  CallTarget getDynamicGetForwarder(Name memberName) =>
       _getterForwarderOfName[memberName] ??= _DynamicForwarderCallTarget(
           translator, _ForwarderKind.Getter, memberName, callingModule);
 
-  CallTarget getDynamicSetForwarder(String memberName) =>
+  CallTarget getDynamicSetForwarder(Name memberName) =>
       _setterForwarderOfName[memberName] ??= _DynamicForwarderCallTarget(
           translator, _ForwarderKind.Setter, memberName, callingModule);
 
-  CallTarget getDynamicInvocationForwarder(String memberName) {
+  CallTarget getDynamicInvocationForwarder(Name memberName) {
     // Add Wasm function to the map before generating the forwarder code, to
     // allow recursive calls in the "call" forwarder.
     var forwarder = _methodForwarderOfName[memberName];
@@ -48,13 +48,13 @@ class DynamicForwarders {
 class _DynamicForwarderCallTarget extends CallTarget {
   final Translator translator;
   final _ForwarderKind _kind;
-  final String memberName;
+  final Name memberName;
   final w.ModuleBuilder callingModule;
 
   _DynamicForwarderCallTarget(
       this.translator, this._kind, this.memberName, this.callingModule)
       : assert(!translator.isDynamicSubmodule ||
-            (memberName == 'call' && _kind == _ForwarderKind.Method)),
+            (memberName.text == 'call' && _kind == _ForwarderKind.Method)),
         super(_kind.functionType(translator));
 
   @override
@@ -95,7 +95,7 @@ class _DynamicForwarderCallTarget extends CallTarget {
 class _DynamicForwarderCodeGenerator extends CodeGenerator {
   final Translator translator;
   final _ForwarderKind _kind;
-  final String memberName;
+  final Name memberName;
   final w.FunctionBuilder function;
 
   _DynamicForwarderCodeGenerator(
@@ -409,7 +409,7 @@ class _DynamicForwarderCodeGenerator extends CodeGenerator {
             b.local_get(namedArgsLocal);
             translator.constants.instantiateConstant(
                 b,
-                SymbolConstant(name, null),
+                translator.symbols.symbolForNamedParameter(name),
                 translator.classInfo[translator.symbolClass]!.nonNullableType);
 
             translator.callReference(
@@ -558,7 +558,7 @@ class _DynamicForwarderCodeGenerator extends CodeGenerator {
           // Value is not a closure
           final callForwarder = translator
               .getDynamicForwardersForModule(b.module)
-              .getDynamicInvocationForwarder("call")
+              .getDynamicInvocationForwarder(Name('call'))
               .function;
           b.local_get(receiverLocal);
           b.local_get(typeArgsLocal);
@@ -728,11 +728,13 @@ void generateDynamicFunctionCall(
 void createInvocationObject(
     Translator translator,
     w.InstructionsBuilder b,
-    String memberName,
+    Name memberName,
     w.Local typeArgsLocal,
     w.Local positionalArgsLocal,
     w.Local namedArgsLocal) {
-  translator.constants.instantiateConstant(b, SymbolConstant(memberName, null),
+  translator.constants.instantiateConstant(
+      b,
+      translator.symbols.methodSymbolFromName(memberName),
       translator.classInfo[translator.symbolClass]!.nonNullableType);
 
   b.local_get(typeArgsLocal);
@@ -748,9 +750,11 @@ void createInvocationObject(
 void createGetterInvocationObject(
   Translator translator,
   w.InstructionsBuilder b,
-  String memberName,
+  Name memberName,
 ) {
-  translator.constants.instantiateConstant(b, SymbolConstant(memberName, null),
+  translator.constants.instantiateConstant(
+      b,
+      translator.symbols.getterSymbolFromName(memberName),
       translator.classInfo[translator.symbolClass]!.nonNullableType);
 
   translator.callReference(translator.invocationGetterFactory.reference, b);
@@ -759,12 +763,12 @@ void createGetterInvocationObject(
 void createSetterInvocationObject(
   Translator translator,
   w.InstructionsBuilder b,
-  String memberName,
+  Name memberName,
   w.Local positionalArgLocal,
 ) {
-  memberName = '$memberName=';
-
-  translator.constants.instantiateConstant(b, SymbolConstant(memberName, null),
+  translator.constants.instantiateConstant(
+      b,
+      translator.symbols.setterSymbolFromName(memberName),
       translator.classInfo[translator.symbolClass]!.nonNullableType);
 
   b.local_get(positionalArgLocal);
diff --git a/pkg/dart2wasm/lib/intrinsics.dart b/pkg/dart2wasm/lib/intrinsics.dart
@@ -2039,8 +2039,8 @@ class Intrinsifier {
             translator,
             b,
             () => b.local_get(closureLocal),
-            () => createInvocationObject(translator, b, "call", typeArgsLocal,
-                posArgsLocal, namedArgsListLocal));
+            () => createInvocationObject(translator, b, Name('call'),
+                typeArgsLocal, posArgsLocal, namedArgsListLocal));
 
       // Error._throw
       case MemberIntrinsic.errorThrow:
diff --git a/pkg/dart2wasm/lib/symbols.dart b/pkg/dart2wasm/lib/symbols.dart
@@ -0,0 +1,64 @@
+// 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:convert';
+
+import 'package:kernel/ast.dart';
+
+class Symbols {
+  final bool minify;
+  Symbols(this.minify);
+
+  SymbolConstant methodSymbolFromName(Name name) {
+    return SymbolConstant(name.text, name.libraryReference);
+  }
+
+  SymbolConstant getterSymbolFromName(Name name) {
+    return SymbolConstant(name.text, name.libraryReference);
+  }
+
+  SymbolConstant setterSymbolFromName(Name name) {
+    return SymbolConstant('${name.text}=', name.libraryReference);
+  }
+
+  SymbolConstant symbolForNamedParameter(String name) {
+    // Named parameters cannot be private.
+    assert(!name.startsWith('_'));
+    return SymbolConstant(name, null);
+  }
+
+  final Map<SymbolConstant, int> symbolOrdinals = {};
+  String getMangledSymbolName(SymbolConstant symbol) {
+    if (minify) {
+      return _intToBase64(
+          symbolOrdinals.putIfAbsent(symbol, () => symbolOrdinals.length));
+    }
+
+    if (symbol.libraryReference == null) {
+      return symbol.name;
+    }
+
+    // We have a private symbol. The symbol must have been constructed using
+    // the `#...` syntax (as all `new Symbol()` or `const Symbol()` are not
+    // considered private - even if they start with "_").
+    //
+    // The symbol must be a private identifier (library symbols like `#a.b.c` do
+    // not have private parts in them).
+    assert(!symbol.name.contains('.') && symbol.name.startsWith('_'));
+    return '${symbol.name}@${symbol.libraryReference!.canonicalName.hashCode}';
+  }
+}
+
+List<int> _intToLittleEndianBytes(int i) {
+  List<int> bytes = [];
+  bytes.add(i & 0xFF);
+  i >>>= 8;
+  while (i != 0) {
+    bytes.add(i & 0xFF);
+    i >>>= 8;
+  }
+  return bytes;
+}
+
+String _intToBase64(int i) => base64.encode(_intToLittleEndianBytes(i));
diff --git a/pkg/dart2wasm/lib/translator.dart b/pkg/dart2wasm/lib/translator.dart
diff --git a/pkg/dart2wasm/lib/types.dart b/pkg/dart2wasm/lib/types.dart
diff --git a/sdk/lib/_internal/wasm/lib/symbol_patch.dart b/sdk/lib/_internal/wasm/lib/symbol_patch.dart
