Change in dart/sdk[master]: [dart2js] Create set of uppercase reserved symbols.
1 view
Skip to first unread message
Joshua Litt (Gerrit)
May 25, 2021, 4:59:41 PM5/25/21
to Stephen Adams, dart2js-te...@google.com, rev...@dartlang.org
Attention is currently required from: Stephen Adams.
Joshua Litt would like Stephen Adams to review this change.
[dart2js] Create set of uppercase reserved symbols.
Change-Id: I07958be1832041a80db9c9dc6281270460d530a9
---
M pkg/compiler/lib/src/js_backend/namer.dart
1 file changed, 80 insertions(+), 0 deletions(-)
diff --git a/pkg/compiler/lib/src/js_backend/namer.dart b/pkg/compiler/lib/src/js_backend/namer.dart
index 2f994bb..33e43f3 100644
--- a/pkg/compiler/lib/src/js_backend/namer.dart
+++ b/pkg/compiler/lib/src/js_backend/namer.dart
@@ -246,6 +246,62 @@
"eval", "arguments"
];
+ /// Note: All of these are already duplicated in [reservedGlobalSymbols]
+ /// below. This set is so [DeferredHolderFinalizer] can use names like:
+ /// [A-Z][_0-9a-zA-Z]* without collisions
+ static const Set<String> reservedUpperCaseGlobalSymbols = const {
+ // Section references are from Ecma-262
+ // (http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-262.pdf)
+
+ // 15.1.1 Value Properties of the Global Object
+ "NaN", "Infinity",
+
+ // 15.1.4 Constructor Properties of the Global Object
+ "Object", "Function", "Array", "String", "Boolean", "Number", "Date",
+ "RegExp", "Symbol", "Error", "EvalError", "RangeError", "ReferenceError",
+ "SyntaxError", "TypeError", "URIError",
+
+ // 15.1.5 Other Properties of the Global Object
+ "Math",
+
+ // Window props (https://developer.mozilla.org/en/DOM/window)
+ "Components",
+
+ // Window methods (https://developer.mozilla.org/en/DOM/window)
+ "GeckoActiveXObject", "QueryInterface", "XPCNativeWrapper",
+ "XPCSafeJSOjbectWrapper",
+
+ // Common browser-defined identifiers not defined in ECMAScript
+ "Debug", "Enumerator", "Global", "Image",
+ "ActiveXObject", "VBArray",
+
+ // Client-side JavaScript identifiers
+ "Anchor", "Applet", "Attr", "Canvas", "CanvasGradient",
+ "CanvasPattern", "CanvasRenderingContext2D", "CDATASection",
+ "CharacterData", "Comment", "CSS2Properties", "CSSRule",
+ "CSSStyleSheet", "Document", "DocumentFragment", "DocumentType",
+ "DOMException", "DOMImplementation", "DOMParser", "Element", "Event",
+ "ExternalInterface", "FlashPlayer", "Form", "Frame", "History",
+ "HTMLCollection", "HTMLDocument", "HTMLElement", "IFrame",
+ "Input", "JSObject", "KeyEvent", "Link", "Location", "MimeType",
+ "MouseEvent", "Navigator", "Node", "NodeList", "Option", "Plugin",
+ "ProcessingInstruction", "Range", "RangeException", "Screen", "Select",
+ "Table", "TableCell", "TableRow", "TableSelection", "Text", "TextArea",
+ "UIEvent", "Window", "XMLHttpRequest", "XMLSerializer",
+ "XPathException", "XPathResult", "XSLTProcessor",
+
+ // These keywords trigger the loading of the java-plugin. For the
+ // next-generation plugin, this results in starting a new Java process.
+ "Packages", "JavaObject", "JavaClass",
+ "JavaArray", "JavaMember",
+
+ // ES6 collections.
+ "Map", "Set",
+
+ // Some additional names
+ "Isolate",
+ };
+
// Symbols that we might be using in our JS snippets.
static const List<String> reservedGlobalSymbols = const <String>[
// Section references are from Ecma-262
@@ -2232,6 +2288,29 @@
Set<String> _jsVariableReservedCache = null;
+ /// Returns true if all reserved names with 2 or more characters long where
+ /// the first character is upper case are in
+ /// [Namer.reservedUpperCaseGlobalSymbols] and all names in that said have
+ /// already been added to [_jsVariableReservedCache].
+ bool _sanityCheckUpperCaseNames(Set<String> reserved) {
+ for (var name in reserved) {
+ var firstChar = name.codeUnitAt(0);
+ if (name.length > 1 &&
+ firstChar >= $A &&
+ firstChar <= $Z &&
+ !Namer.reservedUpperCaseGlobalSymbols.contains(name)) {
+ return false;
+ }
+ }
+
+ for (var name in Namer.reservedUpperCaseGlobalSymbols) {
+ if (!reserved.contains(name)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
/// Names that cannot be used by local variables and parameters.
Set<String> get _jsVariableReserved {
if (_jsVariableReservedCache == null) {
@@ -2243,6 +2322,7 @@
// 26 letters in the alphabet, 25 not counting I.
assert(Namer.reservedGlobalObjectNames.length == 25);
_jsVariableReservedCache.addAll(Namer.reservedGlobalHelperFunctions);
+ assert(_sanityCheckUpperCaseNames(_jsVariableReservedCache));
}
return _jsVariableReservedCache;
}
To view, visit change 201360. To unsubscribe, or for help writing mail filters, visit settings.
Joshua Litt (Gerrit)
May 25, 2021, 4:59:43 PM5/25/21
to dart2js-te...@google.com, rev...@dartlang.org, Stephen Adams
Attention is currently required from: Stephen Adams.
1 comment:
Patchset:
ptal. The alternative to this approach is to move all of the upper case symbols. The advantage of this approach is that it lets us leave the symbols where they logically belong. I don't have strong feelings, and I'm happy to move them if you'd prefer to avoid the duplication.
To view, visit change 201360. To unsubscribe, or for help writing mail filters, visit settings.
Joshua Litt (Gerrit)
May 25, 2021, 5:28:31 PM5/25/21
to dart2js-te...@google.com, rev...@dartlang.org, Stephen Adams
1 comment:
Patchset:
Upon reflection, this is a lot of duplication. I'm going to move the names and then I'll put the cl up for review again. It is a bit unfortunate to have to centralize all of the upper case names into one set, but its probably better than the copypasta.
To view, visit change 201360. To unsubscribe, or for help writing mail filters, visit settings.
Joshua Litt (Gerrit)
May 25, 2021, 6:00:48 PM5/25/21
to dart2js-te...@google.com, rev...@dartlang.org, Stephen Adams
1 comment:
Patchset:
okay ptal, no duplication now.
To view, visit change 201360. To unsubscribe, or for help writing mail filters, visit settings.
Kevin Moore (Gerrit)
May 25, 2021, 7:19:00 PM5/25/21
to Joshua Litt, dart2js-te...@google.com, rev...@dartlang.org, Stephen Adams
Attention is currently required from: Joshua Litt.
2 comments:
Patchset:
DBC
File pkg/compiler/lib/src/js_backend/namer.dart:
Patch Set #2, Line 2281:
if (_jsVariableReservedCache == null) {Great place for collection literals?
To view, visit change 201360. To unsubscribe, or for help writing mail filters, visit settings.
Joshua Litt (Gerrit)
May 27, 2021, 1:24:54 PM5/27/21
to dart2js-te...@google.com, rev...@dartlang.org, Kevin Moore, Stephen Adams
Attention is currently required from: Kevin Moore.
2 comments:
Patchset:
ptal
File pkg/compiler/lib/src/js_backend/namer.dart:
Patch Set #2, Line 2281:
if (_jsVariableReservedCache == null) {Great place for collection literals?
Done
To view, visit change 201360. To unsubscribe, or for help writing mail filters, visit settings.
Stephen Adams (Gerrit)
May 27, 2021, 2:25:02 PM5/27/21
to Joshua Litt, dart2js-te...@google.com, rev...@dartlang.org, Kevin Moore
Attention is currently required from: Kevin Moore, Joshua Litt.
Patch set 3:Code-Review +1
3 comments:
File pkg/compiler/lib/src/js_backend/namer.dart:
Patch Set #3, Line 249:
two lettersnit: if the browser top level environment contained a single-letter upper-case name, it would be here.
Patch Set #3, Line 252:
UpperCasenit: they only start with uppercase.
Capitalized?
reservedGlobalSymbolsThatStartInUpperCase?
or maybe just make the comment more explicitPatch Set #3, Line 443:
if (_jsReserved == null) {nit: could be: return _jsReserved ??= {...
To view, visit change 201360. To unsubscribe, or for help writing mail filters, visit settings.
Joshua Litt (Gerrit)
May 27, 2021, 2:50:32 PM5/27/21
to dart2js-te...@google.com, rev...@dartlang.org, Stephen Adams, Kevin Moore
Attention is currently required from: Kevin Moore.
Patch set 4:Commit-Queue +2
3 comments:
File pkg/compiler/lib/src/js_backend/namer.dart:
Patch Set #3, Line 249:
two letters
nit: if the browser top level environment contained a single-letter upper-case name, it would be her […]
Done
Patch Set #3, Line 252:
UpperCase
nit: they only start with uppercase. […]
Done
Patch Set #3, Line 443:
if (_jsReserved == null) {nit: could be: return _jsReserved ??= {...
Done
To view, visit change 201360. To unsubscribe, or for help writing mail filters, visit settings.
commit-bot@chromium.org (Gerrit)
May 27, 2021, 3:29:24 PM5/27/21
to Joshua Litt, dart2js-te...@google.com, rev...@dartlang.org, Stephen Adams, Kevin Moore
commi...@chromium.org submitted this change.
Approvals:
Stephen Adams: Looks good to me, approved
Joshua Litt: Commit
[dart2js] Create set of uppercase reserved symbols.
Change-Id: I07958be1832041a80db9c9dc6281270460d530a9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201360
Commit-Queue: Joshua Litt <joshu...@google.com>
Reviewed-by: Stephen Adams <s...@google.com>
---
M pkg/compiler/lib/src/js_backend/namer.dart
1 file changed, 93 insertions(+), 51 deletions(-)
diff --git a/pkg/compiler/lib/src/js_backend/namer.dart b/pkg/compiler/lib/src/js_backend/namer.dart
index 2f994bb..8503a49 100644
--- a/pkg/compiler/lib/src/js_backend/namer.dart
+++ b/pkg/compiler/lib/src/js_backend/namer.dart
@@ -246,21 +246,15 @@
"eval", "arguments"
];
- // Symbols that we might be using in our JS snippets.
- static const List<String> reservedGlobalSymbols = const <String>[
+ /// A set of all capitalized global symbols.
+ /// This set is so [DeferredHolderFinalizer] can use names like:
+ /// [A-Z][_0-9a-zA-Z]* without collisions
+ static const Set<String> reservedCapitalizedGlobalSymbols = const {
// Section references are from Ecma-262
// (http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-262.pdf)
// 15.1.1 Value Properties of the Global Object
- "NaN", "Infinity", "undefined",
-
- // 15.1.2 Function Properties of the Global Object
- "eval", "parseInt", "parseFloat", "isNaN", "isFinite",
-
- // 15.1.3 URI Handling Function Properties
- "decodeURI", "decodeURIComponent",
- "encodeURI",
- "encodeURIComponent",
+ "NaN", "Infinity",
// 15.1.4 Constructor Properties of the Global Object
"Object", "Function", "Array", "String", "Boolean", "Number", "Date",
@@ -270,6 +264,61 @@
// 15.1.5 Other Properties of the Global Object
"Math",+ /// Symbols that we might be using in our JS snippets. Some of the symbols in
+ /// these sections are in [reservedGlobalUpperCaseSymbols] above.
+ static const List<String> reservedGlobalSymbols = const <String>[
+ // Section references are from Ecma-262
+ // (http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-262.pdf)
+
+ // 15.1.1 Value Properties of the Global Object
+ "undefined",
+
+ // 15.1.2 Function Properties of the Global Object
+ "eval", "parseInt", "parseFloat", "isNaN", "isFinite",
+
+ // 15.1.3 URI Handling Function Properties
+ "decodeURI", "decodeURIComponent",
+ "encodeURI",
+ "encodeURIComponent",
+
// 10.1.6 Activation Object
"arguments",
@@ -277,7 +326,7 @@
"escape", "unescape",
// Window props (https://developer.mozilla.org/en/DOM/window)
- "applicationCache", "closed", "Components", "content", "controllers",
+ "applicationCache", "closed", "content", "controllers",
"crypto", "defaultStatus", "dialogArguments", "directories",
"document", "frameElement", "frames", "fullScreen", "globalStorage",
"history", "innerHeight", "innerWidth", "length",
@@ -294,15 +343,14 @@
"captureEvents", "clearInterval", "clearTimeout", "close", "confirm",
"disableExternalCapture", "dispatchEvent", "dump",
"enableExternalCapture", "escape", "find", "focus", "forward",
- "GeckoActiveXObject", "getAttention", "getAttentionWithCycleCount",
+ "getAttention", "getAttentionWithCycleCount",
"getComputedStyle", "getSelection", "home", "maximize", "minimize",
"moveBy", "moveTo", "open", "openDialog", "postMessage", "print",
- "prompt", "QueryInterface", "releaseEvents", "removeEventListener",
+ "prompt", "releaseEvents", "removeEventListener",
"resizeBy", "resizeTo", "restore", "routeEvent", "scroll", "scrollBy",
"scrollByLines", "scrollByPages", "scrollTo", "setInterval",
"setResizeable", "setTimeout", "showModalDialog", "sizeToContent",
- "stop", "uuescape", "updateCommands", "XPCNativeWrapper",
- "XPCSafeJSOjbectWrapper",
+ "stop", "uuescape", "updateCommands",
// Mozilla Window event handlers, same cite
"onabort", "onbeforeunload", "onchange", "onclick", "onclose",
@@ -334,34 +382,14 @@
"oncontrolselect", "ondeactivate", "onhelp", "onresizeend",
// Common browser-defined identifiers not defined in ECMAScript
- "event", "external", "Debug", "Enumerator", "Global", "Image",
- "ActiveXObject", "VBArray", "Components",
+ "event", "external",
// Functions commonly defined on Object
"toString", "getClass", "constructor", "prototype", "valueOf",
- // Client-side JavaScript identifiers
- "Anchor", "Applet", "Attr", "Canvas", "CanvasGradient",
- "CanvasPattern", "CanvasRenderingContext2D", "CDATASection",
- "CharacterData", "Comment", "CSS2Properties", "CSSRule",
- "CSSStyleSheet", "Document", "DocumentFragment", "DocumentType",
- "DOMException", "DOMImplementation", "DOMParser", "Element", "Event",
- "ExternalInterface", "FlashPlayer", "Form", "Frame", "History",
- "HTMLCollection", "HTMLDocument", "HTMLElement", "IFrame", "Image",
- "Input", "JSObject", "KeyEvent", "Link", "Location", "MimeType",
- "MouseEvent", "Navigator", "Node", "NodeList", "Option", "Plugin",
- "ProcessingInstruction", "Range", "RangeException", "Screen", "Select",
- "Table", "TableCell", "TableRow", "TableSelection", "Text", "TextArea",
- "UIEvent", "Window", "XMLHttpRequest", "XMLSerializer",
- "XPathException", "XPathResult", "XSLTProcessor",
-
// These keywords trigger the loading of the java-plugin. For the
// next-generation plugin, this results in starting a new Java process.
- "java", "Packages", "netscape", "sun", "JavaObject", "JavaClass",
- "JavaArray", "JavaMember",
-
- // ES6 collections.
- "Map", "Set",
+ "java", "netscape", "sun",
];
// TODO(joshualitt): Stop reserving these names after local naming is updated
@@ -397,7 +425,6 @@
static const List<String> reservedGlobalHelperFunctions = const <String>[
"init",
- "Isolate",
];
static final List<String> userGlobalObjects =
@@ -413,12 +440,7 @@
/// Names that cannot be used by members, top level and static
/// methods.
Set<String> get jsReserved {
- if (_jsReserved == null) {
- _jsReserved = new Set<String>();
- _jsReserved.addAll(javaScriptKeywords);
- _jsReserved.addAll(reservedPropertySymbols);
- }
- return _jsReserved;
+ return _jsReserved ??= {...javaScriptKeywords, ...reservedPropertySymbols};
}
final String stubNameField = r'$stubName';
@@ -2232,17 +2254,37 @@
Set<String> _jsVariableReservedCache = null;
+ /// Returns true if all reserved names with 2 or more characters long where
+ /// the first character is upper case are in
+ /// [Namer.reservedGlobalUpperCaseSymbols] and all names in that said have
+ /// already been added to [_jsVariableReservedCache].
+ bool _sanityCheckUpperCaseNames(Set<String> reserved) {
+ for (var name in reserved) {
+ var firstChar = name.codeUnitAt(0);
+ if (name.length > 1 &&
+ firstChar >= $A &&
+ firstChar <= $Z &&
+ !Namer.reservedCapitalizedGlobalSymbols.contains(name)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
/// Names that cannot be used by local variables and parameters.
Set<String> get _jsVariableReserved {
if (_jsVariableReservedCache == null) {
- _jsVariableReservedCache = new Set<String>();
- _jsVariableReservedCache.addAll(Namer.javaScriptKeywords);
- _jsVariableReservedCache.addAll(Namer.reservedPropertySymbols);
- _jsVariableReservedCache.addAll(Namer.reservedGlobalSymbols);
- _jsVariableReservedCache.addAll(Namer.reservedGlobalObjectNames);
+ _jsVariableReservedCache = {
+ ...Namer.javaScriptKeywords,
+ ...Namer.reservedPropertySymbols,
+ ...Namer.reservedGlobalSymbols,
+ ...Namer.reservedGlobalObjectNames,
+ ...Namer.reservedCapitalizedGlobalSymbols,
+ ...Namer.reservedGlobalHelperFunctions
+ };
// 26 letters in the alphabet, 25 not counting I.
assert(Namer.reservedGlobalObjectNames.length == 25);
- _jsVariableReservedCache.addAll(Namer.reservedGlobalHelperFunctions);
+ assert(_sanityCheckUpperCaseNames(_jsVariableReservedCache));
}
return _jsVariableReservedCache;
}
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
The name of the file: pkg/compiler/lib/src/js_backend/namer.dart
Insertions: 5, Deletions: 8.
```
@@ -248:249, +248:249 @@
- /// A set of all upper case symbols of two letters or longer.
+ /// A set of all capitalized global symbols.
@@ -251:252, +251:252 @@
- static const Set<String> reservedGlobalUpperCaseSymbols = const {
+ static const Set<String> reservedCapitalizedGlobalSymbols = const {
@@ -442:446, +442:443 @@
- if (_jsReserved == null) {
- _jsReserved = {...javaScriptKeywords, ...reservedPropertySymbols};
- }
- return _jsReserved;
+ return _jsReserved ??= {...javaScriptKeywords, ...reservedPropertySymbols};
@@ -2269:2270, +2266:2267 @@
- !Namer.reservedGlobalUpperCaseSymbols.contains(name)) {
+ !Namer.reservedCapitalizedGlobalSymbols.contains(name)) {
@@ -2284:2285, +2281:2282 @@
- ...Namer.reservedGlobalUpperCaseSymbols,
+ ...Namer.reservedCapitalizedGlobalSymbols,
```
To view, visit change 201360. To unsubscribe, or for help writing mail filters, visit settings.
Dart CI (Gerrit)
May 27, 2021, 3:57:14 PM5/27/21
to commi...@chromium.org, Joshua Litt, dart2js-te...@google.com, rev...@dartlang.org, Stephen Adams, Kevin Moore
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/6a4bd42396c4cc9b73cf210e563d5245eb351a8e
Kevin Moore (Gerrit)
May 27, 2021, 5:45:57 PM5/27/21
to commi...@chromium.org, Joshua Litt, dart2js-te...@google.com, rev...@dartlang.org, Dart CI, Stephen Adams
Attention is currently required from: Joshua Litt.
3 comments:
Patchset:
More being nitty :-)
File pkg/compiler/lib/src/js_backend/namer.dart:
Patch Set #5, Line 443:
return _jsReserved ??= {...javaScriptKeywords, ...reservedPropertySymbols};couldn't this just be a lazy final?
Patch Set #5, Line 2275:
Set<String> get _jsVariableReserved {ditto around final?
To view, visit change 201360. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages