Skip to content

Commit 38f43f7

Browse files
rakudramaCommit Queue
authored andcommitted
[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 <[email protected]> Commit-Queue: Stephen Adams <[email protected]>
1 parent ab4f935 commit 38f43f7

File tree

2 files changed

+17
-46
lines changed

2 files changed

+17
-46
lines changed

pkg/compiler/lib/src/js_backend/deferred_holder_expression.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -438,9 +438,9 @@ class DeferredHolderExpressionFinalizerImpl
438438

439439
/// Returns true if [element] is stored in the static state holder
440440
/// ([staticStateHolder]). We intend to store only mutable static state
441-
/// there, whereas constants are stored in 'C'. Functions, accessors,
442-
/// classes, etc. are stored in one of the other objects in
443-
/// [reservedGlobalObjectNames].
441+
/// there, whereas constants, functions, accessors,
442+
/// classes, etc. are stored in one of the other 'holder' objects.
443+
// TODO(60939): Have multiple holders.
444444
bool _isPropertyOfStaticStateHolder(MemberEntity element) {
445445
// TODO(ahe): Make sure this method's documentation is always true and
446446
// remove the word "intend".

pkg/compiler/lib/src/js_backend/namer.dart

Lines changed: 14 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1824,14 +1824,14 @@ abstract class ModularNamer {
18241824

18251825
/// Returns a variable use for accessing interceptors.
18261826
///
1827-
/// This is one of the [reservedGlobalObjectNames]
1827+
/// This is the name of a 'holder'.
18281828
js_ast.Expression readGlobalObjectForInterceptors() {
18291829
return DeferredHolderExpression.forInterceptors();
18301830
}
18311831

18321832
/// Returns a variable use for accessing the class [element].
18331833
///
1834-
/// This is one of the [reservedGlobalObjectNames]
1834+
/// This is the name of a 'holder'.
18351835
js_ast.Expression readGlobalObjectForClass(ClassEntity element) {
18361836
return DeferredHolderExpression(
18371837
DeferredHolderExpressionKind.globalObjectForClass,
@@ -1840,6 +1840,8 @@ abstract class ModularNamer {
18401840
}
18411841

18421842
/// Returns a variable use for accessing the member [element].
1843+
///
1844+
/// This is the name of a 'holder'.
18431845
js_ast.Expression readGlobalObjectForMember(MemberEntity element) {
18441846
return DeferredHolderExpression(
18451847
DeferredHolderExpressionKind.globalObjectForMember,
@@ -2002,7 +2004,6 @@ abstract class ModularNamer {
20022004
...javaScriptKeywords,
20032005
...reservedPropertySymbols,
20042006
...reservedGlobalSymbols,
2005-
...reservedGlobalObjectNames,
20062007
...reservedCapitalizedGlobalSymbols,
20072008
...reservedGlobalHelperFunctions,
20082009
};
@@ -2026,8 +2027,6 @@ abstract class ModularNamer {
20262027

20272028
/// Names that cannot be used by local variables and parameters.
20282029
Set<String> get _jsVariableReserved {
2029-
// 26 letters in the alphabet, 25 not counting I.
2030-
assert(reservedGlobalObjectNames.length == 25);
20312030
assert(_sanityCheckUpperCaseNames(_jsVariableReservedCache));
20322031
return _jsVariableReservedCache;
20332032
}
@@ -2039,7 +2038,9 @@ abstract class ModularNamer {
20392038
/// same name for two different inputs.
20402039
String safeVariableName(String name) {
20412040
name = name.replaceAll('#', '_');
2042-
if (_jsVariableReserved.contains(name) || name.startsWith(r'$')) {
2041+
if (_jsVariableReserved.contains(name) ||
2042+
name.startsWith(r'$') ||
2043+
_holderNameRE.hasMatch(name)) {
20432044
return '\$$name';
20442045
}
20452046
return name;
@@ -2664,46 +2665,16 @@ const List<String> reservedGlobalSymbols = [
26642665
"java", "netscape", "sun",
26652666
];
26662667

2667-
// TODO(joshualitt): Stop reserving these names after local naming is updated
2668-
// to use frequencies.
2669-
const List<String> reservedGlobalObjectNames = [
2670-
"A",
2671-
"B",
2672-
"C", // Global object for *C*onstants.
2673-
"D",
2674-
"E",
2675-
"F",
2676-
"G",
2677-
"H", // Global object for internal (*H*elper) libraries.
2678-
// I is used for used for the Isolate function.
2679-
"J", // Global object for the interceptor library.
2680-
"K",
2681-
"L",
2682-
"M",
2683-
"N",
2684-
"O",
2685-
"P", // Global object for other *P*latform libraries.
2686-
"Q",
2687-
"R",
2688-
"S",
2689-
"T",
2690-
"U",
2691-
"V",
2692-
"W", // Global object for *W*eb libraries (dart:html).
2693-
"X",
2694-
"Y",
2695-
"Z",
2696-
];
2668+
/// Part of the local variable space is reserved for 'holders'. Holder names
2669+
/// start with an uppercase letter and are short, since they are always
2670+
/// 'minified'. There is no fixed name for a given holder, each 'part' file has
2671+
/// its own local name for the shared holder objects it references, starting
2672+
/// with single letter names. In large programs there are a few thousand
2673+
/// holders, so recognizing up to four characters (6.19M names) seems safe.
2674+
final RegExp _holderNameRE = RegExp(r'^[A-Z].{0,3}$');
26972675

26982676
const List<String> reservedGlobalHelperFunctions = ["init"];
26992677

2700-
final List<String> userGlobalObjects = List.from(reservedGlobalObjectNames)
2701-
..remove('C')
2702-
..remove('H')
2703-
..remove('J')
2704-
..remove('P')
2705-
..remove('W');
2706-
27072678
final RegExp _identifierStartRE = RegExp(r'[A-Za-z_$]');
27082679
final RegExp _nonIdentifierRE = RegExp(r'[^A-Za-z0-9_$]');
27092680

0 commit comments

Comments
 (0)