[dart2js] Avoid all potential holder names for local variables 'Tested' by inspection. Type variable names like `T1` are now also 'escaped', e.g. to `$T1`. (It is quite difficult to come up with a reasonable size repro since the original problem it requires a deferred loaded 'part' file that refers to entities defined by several dozen other deferred loaded part files.) Bug: 60891 Change-Id: I32c370c591c8fa4c67a15fb633d02fba73bacf5a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/435060 Reviewed-by: Nate Biggs <natebiggs@google.com> Commit-Queue: Stephen Adams <sra@google.com>
diff --git a/pkg/compiler/lib/src/js_backend/deferred_holder_expression.dart b/pkg/compiler/lib/src/js_backend/deferred_holder_expression.dart index be85c0c..01161ff 100644 --- a/pkg/compiler/lib/src/js_backend/deferred_holder_expression.dart +++ b/pkg/compiler/lib/src/js_backend/deferred_holder_expression.dart
@@ -438,9 +438,9 @@ /// Returns true if [element] is stored in the static state holder /// ([staticStateHolder]). We intend to store only mutable static state - /// there, whereas constants are stored in 'C'. Functions, accessors, - /// classes, etc. are stored in one of the other objects in - /// [reservedGlobalObjectNames]. + /// there, whereas constants, functions, accessors, + /// classes, etc. are stored in one of the other 'holder' objects. + // TODO(60939): Have multiple holders. bool _isPropertyOfStaticStateHolder(MemberEntity element) { // TODO(ahe): Make sure this method's documentation is always true and // remove the word "intend".
diff --git a/pkg/compiler/lib/src/js_backend/namer.dart b/pkg/compiler/lib/src/js_backend/namer.dart index e9d87b2..5fa976e 100644 --- a/pkg/compiler/lib/src/js_backend/namer.dart +++ b/pkg/compiler/lib/src/js_backend/namer.dart
@@ -1824,14 +1824,14 @@ /// Returns a variable use for accessing interceptors. /// - /// This is one of the [reservedGlobalObjectNames] + /// This is the name of a 'holder'. js_ast.Expression readGlobalObjectForInterceptors() { return DeferredHolderExpression.forInterceptors(); } /// Returns a variable use for accessing the class [element]. /// - /// This is one of the [reservedGlobalObjectNames] + /// This is the name of a 'holder'. js_ast.Expression readGlobalObjectForClass(ClassEntity element) { return DeferredHolderExpression( DeferredHolderExpressionKind.globalObjectForClass, @@ -1840,6 +1840,8 @@ } /// Returns a variable use for accessing the member [element]. + /// + /// This is the name of a 'holder'. js_ast.Expression readGlobalObjectForMember(MemberEntity element) { return DeferredHolderExpression( DeferredHolderExpressionKind.globalObjectForMember, @@ -2002,7 +2004,6 @@ ...javaScriptKeywords, ...reservedPropertySymbols, ...reservedGlobalSymbols, - ...reservedGlobalObjectNames, ...reservedCapitalizedGlobalSymbols, ...reservedGlobalHelperFunctions, }; @@ -2026,8 +2027,6 @@ /// Names that cannot be used by local variables and parameters. Set<String> get _jsVariableReserved { - // 26 letters in the alphabet, 25 not counting I. - assert(reservedGlobalObjectNames.length == 25); assert(_sanityCheckUpperCaseNames(_jsVariableReservedCache)); return _jsVariableReservedCache; } @@ -2039,7 +2038,9 @@ /// same name for two different inputs. String safeVariableName(String name) { name = name.replaceAll('#', '_'); - if (_jsVariableReserved.contains(name) || name.startsWith(r'$')) { + if (_jsVariableReserved.contains(name) || + name.startsWith(r'$') || + _holderNameRE.hasMatch(name)) { return '\$$name'; } return name; @@ -2664,46 +2665,16 @@ "java", "netscape", "sun", ]; -// TODO(joshualitt): Stop reserving these names after local naming is updated -// to use frequencies. -const List<String> reservedGlobalObjectNames = [ - "A", - "B", - "C", // Global object for *C*onstants. - "D", - "E", - "F", - "G", - "H", // Global object for internal (*H*elper) libraries. - // I is used for used for the Isolate function. - "J", // Global object for the interceptor library. - "K", - "L", - "M", - "N", - "O", - "P", // Global object for other *P*latform libraries. - "Q", - "R", - "S", - "T", - "U", - "V", - "W", // Global object for *W*eb libraries (dart:html). - "X", - "Y", - "Z", -]; +/// Part of the local variable space is reserved for 'holders'. Holder names +/// start with an uppercase letter and are short, since they are always +/// 'minified'. There is no fixed name for a given holder, each 'part' file has +/// its own local name for the shared holder objects it references, starting +/// with single letter names. In large programs there are a few thousand +/// holders, so recognizing up to four characters (6.19M names) seems safe. +final RegExp _holderNameRE = RegExp(r'^[A-Z].{0,3}$'); const List<String> reservedGlobalHelperFunctions = ["init"]; -final List<String> userGlobalObjects = List.from(reservedGlobalObjectNames) - ..remove('C') - ..remove('H') - ..remove('J') - ..remove('P') - ..remove('W'); - final RegExp _identifierStartRE = RegExp(r'[A-Za-z_$]'); final RegExp _nonIdentifierRE = RegExp(r'[^A-Za-z0-9_$]');