Address feedback from plenary #77

bakkot · 2024-04-09T05:07:53Z

Commits should be reviewed individually. Summary:

use \$ etc where possibly; this is not possible for all punctuators, but it is for some
also escape line terminators; this was just an oversight on my part
use \n etc where possible
if given a lone surrogate, escape it, so that if there is someday a non-BMP whitespace character, and we get x-mode RegExps, and someone writes new RegExp(RegExp.escape('\uHEAD') + RegExp.escape('\uTAIL'), 'x') where \uHEAD\uTAIL encodes that non-BMP whitespace character, the resulting RegExp will match the character instead of being interpreted as whitespace and therefore (in x-mode) ignored
hex-escape ASCII letters at the start of the string so that new RegExp('\c' + RegExp.escape('Z')) will either be an error (in u- or v-mode RegExps) or match the string '\\cZ' (in other RegExps); this also has the effect that the result cannot combine with a preceding \x or \u when the first character is A-F or a-f. Fixes Which leading characters should be escaped? #66.

bakkot · 2024-04-09T05:08:31Z

cc @erights @waldemarhorwat @gibson042 Please take a look.

ljharb · 2024-04-09T05:12:55Z

spec.emu

@@ -62,7 +62,7 @@ contributors:
            1. Return the string-concatenation of 0x005C (REVERSE SOLIDUS) and the string in the "ControlEscape" column of the row whose "Code Point" column contains _c_.
          1. Let _otherPunctuators_ be the string-concatenation of *",-=<>#&!%:;@~'`"* and the code unit 0x0022 (QUOTATION MARK).
          1. Let _toEscape_ be StringToCodePoints(_otherPunctuators_).
-          1. If _toEscape_ contains _c_ or _c_ is matched by |WhiteSpace| or |LineTerminator|, then
+          1. If _toEscape_ contains _c_, _c_ is matched by |WhiteSpace| or |LineTerminator|, or _c_ is in the inclusive interval from U+D800 to U+DFFF (i.e. _c_ is a leading surrogate or trailing surrogate), then


1. If _toEscape_ contains _c_, _c_ is matched by |WhiteSpace| or |LineTerminator|, or _c_ is a leading surrogate or a trailing surrogate, then

Strictly speaking, leading and trailing surrogates are defined to be code units, not code points. I figure it's close enough for the parenthetical, but not the actual algorithm step. I could change the parenthetical to be "c is the code point corresponding to a leading surrogate or trailing surrogate" but that's getting pretty wordy.

other than the oxford comma, this suggestion is a nit, so i'm ambivalent

spec.emu

erights · 2024-04-09T21:33:04Z

How do I see a rendered form of this?

bakkot · 2024-04-09T22:00:20Z

I've put up a rendering here.

erights · 2024-04-09T22:13:15Z

Thanks. FWIW LGTM, but I delegate an approval decision to @gibson042 who understands this better than I do.

erights · 2024-04-09T22:13:51Z

Thanks for the rendering. Cut down the review effort for me by a huge factor.

gibson042

Nice catch on LineTerminator! I still prefer the universally applicable \x…/\u… approach, but this does work (even if it could get stale).

spec.emu

gibson042 · 2024-04-12T04:09:47Z

spec.emu

+          1. If _c_ is matched by |SyntaxCharacter| or _c_ is U+002F (SOLIDUS), then
+            1. Return the string-concatenation of 0x005C (REVERSE SOLIDUS) and UTF16EncodeCodePoint(_c_).


This strikes me as a point-in-time snapshot, and I'm not a fan because it will be weird if e.g. \@ becomes a valid escape in the future. My preference remains the universally applicable \x…/\u… approach. But that said, there is no technical issue here; merely an aesthetic one.

jridgewell · 2024-04-12T22:53:06Z

spec.emu

-              1. NOTE: Escaping a leading digit ensures that output corresponds with pattern text which may be used after a `\0` character escape or a |DecimalEscape| such as `\1` and still match _S_ rather than be interpreted as an extension of the preceding escape sequence.
-              1. Set _escaped_ to the string-concatenation of _escaped_, the code unit 0x005C (REVERSE SOLIDUS), *"x3"*, and the code unit whose numeric value is the numeric value of _c_.
+            1. If _escaped_ is the empty String, and _c_ is matched by |DecimalDigit| or |AsciiLetter|, then
+              1. NOTE: Escaping a leading digit ensures that output corresponds with pattern text which may be used after a `\0` character escape or a |DecimalEscape| such as `\1` and still match _S_ rather than be interpreted as an extension of the preceding escape sequence. Escaping a leading ASCII letter does the same for the context after `\c`.


Nit: this sentence is too long to not use a parenthetical.

Suggested change

1. NOTE: Escaping a leading digit ensures that output corresponds with pattern text which may be used after a `\0` character escape or a |DecimalEscape| such as `\1` and still match _S_ rather than be interpreted as an extension of the preceding escape sequence. Escaping a leading ASCII letter does the same for the context after `\c`.

1. NOTE: Escaping a leading digit ensures that output corresponds with pattern text, which may be used after a `\0` character escape or a |DecimalEscape| such as `\1`, and still match _S_ rather than be interpreted as an extension of the preceding escape sequence. Escaping a leading ASCII letter does the same for the context after `\c`.

Those commas read as ungrammatical to me. In particular the second comma cuts a clause in the middle - the pattern text "maybe used after \0 [...] and still match S". Open to other rephrasing here but I don't like this particular suggestion.

jridgewell · 2024-04-12T22:54:23Z

spec.emu

-              1. NOTE: Escaping a leading digit ensures that output corresponds with pattern text which may be used after a `\0` character escape or a |DecimalEscape| such as `\1` and still match _S_ rather than be interpreted as an extension of the preceding escape sequence.
-              1. Set _escaped_ to the string-concatenation of _escaped_, the code unit 0x005C (REVERSE SOLIDUS), *"x3"*, and the code unit whose numeric value is the numeric value of _c_.
+            1. If _escaped_ is the empty String, and _c_ is matched by |DecimalDigit| or |AsciiLetter|, then
+              1. NOTE: Escaping a leading digit ensures that output corresponds with pattern text which may be used after a `\0` character escape or a |DecimalEscape| such as `\1` and still match _S_ rather than be interpreted as an extension of the preceding escape sequence. Escaping a leading ASCII letter does the same for the context after `\c`.


Also, TIL \c0 is a valid escape. This also prevents new RegExp("\\" + escape('n')) from combining.

spec.emu

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [core-js](https://github.com/zloirock/core-js) | dependencies | minor | [`3.36.1` -> `3.37.1`](https://renovatebot.com/diffs/npm/core-js/3.36.1/3.37.1) | --- ### Release Notes <details> <summary>zloirock/core-js (core-js)</summary> ### [`v3.37.1`](https://github.com/zloirock/core-js/blob/HEAD/CHANGELOG.md#3371---20240514) [Compare Source](zloirock/core-js@v3.37.0...v3.37.1) - Changes [v3.37.0...v3.37.1](zloirock/core-js@v3.37.0...v3.37.1) - Fixed [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) feature detection for some specific cases - Compat data improvements: - [`Set` methods proposal](https://github.com/tc39/proposal-set-methods) added and marked as [supported from FF 127](https://bugzilla.mozilla.org/show_bug.cgi?id=1868423) - [`Symbol.dispose`](https://github.com/tc39/proposal-explicit-resource-management) added and marked as supported from V8 ~ Chromium 125 - [`Math.f16round` and `DataView.prototype.{ getFloat16, setFloat16 }`](https://github.com/tc39/proposal-float16array) added and marked as [supported from Deno 1.43](denoland/deno#23490) - [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) added and marked as [supported from Chromium 126](https://chromestatus.com/feature/6301071388704768) - [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) added and marked as [supported from NodeJS 22.0](nodejs/node#52280) - [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) added and marked as [supported from Deno 1.43](denoland/deno#23318) - Added [Rhino 1.7.15](https://github.com/mozilla/rhino/releases/tag/Rhino1\_7\_15\_Release) compat data, many features marked as supported - Added [NodeJS 22.0](https://nodejs.org/en/blog/release/v22.0.0) compat data mapping - Added [Deno 1.43](https://github.com/denoland/deno/releases/tag/v1.43.0) compat data mapping - Added Electron 31 compat data mapping - Updated [Opera Android 82](https://forums.opera.com/topic/71513/opera-for-android-82) compat data mapping - Added Samsung Internet 26 compat data mapping - Added Oculus Quest Browser 33 compat data mapping ### [`v3.37.0`](https://github.com/zloirock/core-js/blob/HEAD/CHANGELOG.md#3370---20240417) [Compare Source](zloirock/core-js@v3.36.1...v3.37.0) - Changes [v3.36.1...v3.37.0](zloirock/core-js@v3.36.1...v3.37.0) - [New `Set` methods proposal](https://github.com/tc39/proposal-set-methods): - Built-ins: - `Set.prototype.intersection` - `Set.prototype.union` - `Set.prototype.difference` - `Set.prototype.symmetricDifference` - `Set.prototype.isSubsetOf` - `Set.prototype.isSupersetOf` - `Set.prototype.isDisjointFrom` - Moved to stable ES, [April 2024 TC39 meeting](tc39/proposals@bda5a6b) - Added `es.` namespace modules, `/es/` and `/stable/` namespaces entries - [Explicit Resource Management stage 3 proposal](https://github.com/tc39/proposal-explicit-resource-management): - Some minor updates like [explicit-resource-management/217](tc39/proposal-explicit-resource-management#217) - Added [`Math.sumPrecise` stage 2.7 proposal](https://github.com/tc39/proposal-math-sum/): - Built-ins: - `Math.sumPrecise` - [`Promise.try` proposal](https://github.com/tc39/proposal-promise-try): - Built-ins: - `Promise.try` - Added optional arguments support, [promise-try/16](tc39/proposal-promise-try#16) - Moved to stage 2.7, [April 2024 TC39 meeting](tc39/proposals@301fc9c) - [`RegExp.escape` stage 2 proposal](https://github.com/tc39/proposal-regex-escaping): - Moved to hex-escape semantics, [regex-escaping/67](tc39/proposal-regex-escaping#67) - It's not the final change of the way of escaping, waiting for [regex-escaping/77](tc39/proposal-regex-escaping#77) soon - [Pattern matching stage 1 proposal](https://github.com/tc39/proposal-pattern-matching): - Built-ins: - `Symbol.customMatcher` - Once again, [the used well-known symbol was renamed](tc39/proposal-pattern-matching#295) - Added new entries for that - Added [Extractors stage 1 proposal](https://github.com/tc39/proposal-extractors): - Built-ins: - `Symbol.customMatcher` - Since the `Symbol.customMatcher` well-known symbol from the pattern matching proposal is also used in the exactors proposal, added an entry also for this proposal - Added [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse), [url/825](whatwg/url#825) - Engines bugs fixes: - Added a fix of [Safari `{ Object, Map }.groupBy` bug that does not support iterable primitives](https://bugs.webkit.org/show_bug.cgi?id=271524) - Added a fix of [Safari bug with double call of constructor in `Array.fromAsync`](https://bugs.webkit.org/show_bug.cgi?id=271703) - Compat data improvements: - [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) added and marked as supported [from FF 126](https://bugzilla.mozilla.org/show_bug.cgi?id=1887611) - [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) added and marked as supported [from Bun 1.1.4](oven-sh/bun#10129) - [`URL.canParse`](https://url.spec.whatwg.org/#dom-url-canparse) fixed and marked as supported [from Bun 1.1.0](oven-sh/bun#9710) - [New `Set` methods](https://github.com/tc39/proposal-set-methods) fixed in JavaScriptCore and marked as supported from Bun 1.1.1 - Added Opera Android 82 compat data mapping </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.vylpes.xyz/RabbitLabs/Droplet/pulls/306 Co-authored-by: Renovate Bot <renovate@vylpes.com> Co-committed-by: Renovate Bot <renovate@vylpes.com>

bakkot added 5 commits April 8, 2024 20:49

use identity escapes where possible

4221ff9

also escape line terminators

07fc1bd

use control escapes where possible

b51ecb4

also escape lone surrogates

eef281a

also escape leading ascii letters

27eee05

bakkot force-pushed the address-feedback branch 2 times, most recently from 7843108 to a839978 Compare April 9, 2024 05:13

ljharb reviewed Apr 9, 2024

View reviewed changes

ljharb requested review from erights, gibson042 and waldemarhorwat April 9, 2024 15:22

gibson042 approved these changes Apr 12, 2024

View reviewed changes

jridgewell approved these changes Apr 12, 2024

View reviewed changes

ljharb force-pushed the address-feedback branch from 82c6d6f to 27eee05 Compare May 13, 2024 05:03

ljharb approved these changes May 13, 2024

View reviewed changes

ljharb merged commit 2481aa8 into tc39:main May 13, 2024
1 check passed

bakkot deleted the address-feedback branch May 13, 2024 05:35

gibson042 mentioned this pull request May 16, 2024

Path to Stage 4! #58

Open

25 tasks

BuloZB mentioned this pull request May 19, 2024

[Snyk] Upgrade core-js from 3.1.4 to 3.37.0 BuloZB/ClassicGuildBank#2

Open

dariosalvi78 mentioned this pull request May 19, 2024

[Snyk] Upgrade core-js from 3.36.1 to 3.37.0 Mobistudy/MobistudyWeb#408

Open

cb-sl mentioned this pull request May 20, 2024

[Snyk-dev] Upgrade core-js from 3.22.8 to 3.37.0 cb-sl/cb-juice-shop#5

Open

julitabarelkowska mentioned this pull request May 20, 2024

[Snyk] Upgrade core-js from 3.15.1 to 3.37.0 julitabarelkowska/bridge#8

Open

vinit-aikido mentioned this pull request May 20, 2024

[Snyk] Upgrade core-js-bundle from 3.19.3 to 3.37.0 vinit-aikido/terragoat#37

Open

Shimizu-Kanaden-sys mentioned this pull request May 20, 2024

[Snyk] Upgrade core-js from 3.22.8 to 3.37.0 Shimizu-Kanaden-sys/juice-shop#10

Open

olokotoh mentioned this pull request May 21, 2024

[Snyk] Upgrade core-js from 3.23.2 to 3.37.0 olokotoh/semaphore#1

Open

snyk-io bot mentioned this pull request May 21, 2024

[Snyk] Upgrade core-js from 3.9.0 to 3.37.0 Hawthorne001/bit#5

Open

AndrzejSmrokowski mentioned this pull request May 22, 2024

[Snyk] Upgrade core-js from 3.35.1 to 3.37.0 AndrzejSmrokowski/Minesweeper.com-Symfony-Vue.js#4

Open

marekful mentioned this pull request May 22, 2024

[Snyk] Upgrade core-js from 3.30.1 to 3.37.0 marekful/webscp#109

Open

snyk-io bot mentioned this pull request May 23, 2024

[Snyk] Upgrade core-js from 3.36.0 to 3.37.0 Hawthorne001/blockscout#4

Open

patrick-zvenyika mentioned this pull request May 23, 2024

[Snyk] Upgrade core-js from 3.36.1 to 3.37.0 patrick-zvenyika/EcoSto#2

Open

nejidevelops mentioned this pull request May 24, 2024

[Snyk] Upgrade core-js from 3.9.0 to 3.37.0 nejidevelops/my-static-web-app-and-api#12

Open

JosanaDH mentioned this pull request May 24, 2024

[Snyk] Upgrade core-js from 3.22.8 to 3.37.0 insightsTest/juice-shopFork#4

Open

samul-1 mentioned this pull request May 24, 2024

[Snyk] Upgrade core-js from 3.9.1 to 3.37.0 samul-1/sai_exams_frontend#135

Open

Kypaku mentioned this pull request May 24, 2024

[Snyk] Upgrade core-js from 3.29.1 to 3.37.0 Kypaku/vue-gpt-playground#107

Open

FlorentinTh mentioned this pull request May 24, 2024

[Snyk] Upgrade core-js from 3.23.3 to 3.37.0 FlorentinTh/LE2ML-Windowing-Module#347

Open

This was referenced May 24, 2024

[Snyk] Upgrade core-js from 3.32.0 to 3.37.0 TheAutisticTechie/browser#46

Open

[Snyk] Upgrade core-js from 3.22.5 to 3.37.0 TheAutisticTechie/web#63

Open

FlorentinTh mentioned this pull request May 24, 2024

[Snyk] Upgrade core-js from 3.23.3 to 3.37.0 FlorentinTh/LE2ML-FeatureExtractor-Module#344

Open

nejidevelops mentioned this pull request May 24, 2024

[Snyk] Upgrade core-js from 3.14.0 to 3.37.0 nejidevelops/my-static-web-app#21

Open

S0nG0ku0 mentioned this pull request May 25, 2024

[Snyk] Upgrade core-js from 3.27.1 to 3.37.0 S0nG0ku0/music-social-network-frontend#79

Open

larkinwang20 mentioned this pull request May 25, 2024

[Snyk] Upgrade core-js from 3.15.2 to 3.37.0 larkinwang20/ssg--vue23-#52

Open

HarmlessHarm mentioned this pull request May 25, 2024

[Snyk] Upgrade core-js from 3.36.1 to 3.37.0 HarmlessKey/Shieldmaiden#238

Closed

larkinwang20 mentioned this pull request May 25, 2024

[Snyk] Upgrade core-js from 3.13.1 to 3.37.0 larkinwang20/ssg--vue23-#53

Open

merfu mentioned this pull request May 25, 2024

[Snyk] Upgrade core-js from 2.6.12 to 3.37.0 merfu/aws-serverless-workshops#4

Open

b34c0n5 mentioned this pull request May 25, 2024

[Snyk] Upgrade core-js from 3.32.2 to 3.37.0 b34c0n5/monkey#36

Open

gaohongwei11 mentioned this pull request May 25, 2024

[Snyk] Upgrade core-js from 3.34.0 to 3.37.0 gaohongwei11/gemini#4

Open

phen1 mentioned this pull request May 25, 2024

[Snyk] Upgrade core-js from 3.25.2 to 3.37.0 phen1/country-catalog-assignment#4

Open

galoget mentioned this pull request May 26, 2024

[Snyk] Upgrade core-js from 3.9.0 to 3.37.0 galoget/pwndoc-ng#2

Open

This was referenced May 26, 2024

[Snyk] Upgrade core-js from 3.6.5 to 3.37.0 OKEAMAH/renovate-profiler#8

Open

[Snyk] Upgrade core-js from 3.36.1 to 3.37.0 OKEAMAH/blockscout#5

Open

[Snyk] Upgrade core-js from 3.36.1 to 3.37.0 OKEAMAH/Temp#32

Open

BitcoinOutput mentioned this pull request May 28, 2024

[Snyk] Upgrade core-js from 3.26.0 to 3.37.0 BitcoinOutput/iaptos-nft-random-mint#278

Open

bsimmons123 mentioned this pull request May 28, 2024

[Snyk] Upgrade core-js from 3.36.1 to 3.37.0 bsimmons123/invstore-jvm#23

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address feedback from plenary #77

Address feedback from plenary #77

bakkot commented Apr 9, 2024 •

edited

bakkot commented Apr 9, 2024

ljharb Apr 9, 2024

bakkot Apr 9, 2024

ljharb Apr 9, 2024

erights commented Apr 9, 2024

bakkot commented Apr 9, 2024

erights commented Apr 9, 2024

erights commented Apr 9, 2024

gibson042 left a comment •

edited

gibson042 Apr 12, 2024

jridgewell Apr 12, 2024

bakkot May 4, 2024

jridgewell Apr 12, 2024

		1. If _c_ is matched by \|SyntaxCharacter\| or _c_ is U+002F (SOLIDUS), then
		1. Return the string-concatenation of 0x005C (REVERSE SOLIDUS) and UTF16EncodeCodePoint(_c_).

	1. NOTE: Escaping a leading digit ensures that output corresponds with pattern text which may be used after a `\0` character escape or a \|DecimalEscape\| such as `\1` and still match _S_ rather than be interpreted as an extension of the preceding escape sequence. Escaping a leading ASCII letter does the same for the context after `\c`.
	1. NOTE: Escaping a leading digit ensures that output corresponds with pattern text, which may be used after a `\0` character escape or a \|DecimalEscape\| such as `\1`, and still match _S_ rather than be interpreted as an extension of the preceding escape sequence. Escaping a leading ASCII letter does the same for the context after `\c`.

Address feedback from plenary #77

Address feedback from plenary #77

Conversation

bakkot commented Apr 9, 2024 • edited

bakkot commented Apr 9, 2024

ljharb Apr 9, 2024

Choose a reason for hiding this comment

bakkot Apr 9, 2024

Choose a reason for hiding this comment

ljharb Apr 9, 2024

Choose a reason for hiding this comment

erights commented Apr 9, 2024

bakkot commented Apr 9, 2024

erights commented Apr 9, 2024

erights commented Apr 9, 2024

gibson042 left a comment • edited

Choose a reason for hiding this comment

gibson042 Apr 12, 2024

Choose a reason for hiding this comment

jridgewell Apr 12, 2024

Choose a reason for hiding this comment

bakkot May 4, 2024

Choose a reason for hiding this comment

jridgewell Apr 12, 2024

Choose a reason for hiding this comment

bakkot commented Apr 9, 2024 •

edited

gibson042 left a comment •

edited