| 00:54 | <TimothyGu> | rkirsling: (1.875).toString(2) is a thing though |
| 00:55 | <rkirsling> | yeah, totally |
| 00:55 | <rkirsling> | these seem like nice little things that could be proposed |
| 15:51 | <bradleymeck> | reading https://tc39.es/ecma262/#sec-promise.race , it doesn't seem like things should be holding onto all arguments if it resolves , I think this should be explicit that an engine is expected to not tie the lifetimes of the arguments together given potential things like weakrefs observing this |
| 15:53 | <bradleymeck> | not sure how to phrase things |
| 15:53 | <devsnek> | bradleymeck: nothing is holding onto the arguments even if nothing resolves |
| 15:53 | <bradleymeck> | in the spec, sure |
| 15:53 | <devsnek> | and therefore implementations aren't required to do so either |
| 15:54 | <devsnek> | this spec text is about as close as you can get to `return new Promise((resolve, reject) => { for (const promise of iterable) { promise.then(resolve, reject); } })`` |
| 15:56 | <bradleymeck> | but they do |
| 15:57 | <Bakkot> | bradleymeck there's a _lot_ of places where the spec holds onto things but engines don't need to |
| 15:57 | <devsnek> | sounds like engines need to be updated |
| 15:58 | <Bakkot> | (WeakMaps are my favorite example; the spec for WeakMaps does not actually delete things from the internal data list when they get GC'd, but since it does not expose the internal list it doesn't matter) |
| 15:58 | <devsnek> | bradleymeck: was promise.race in V8 having this problem? I can take a look at it later today if so |
| 15:59 | <bradleymeck> | Bakkot: yep |
| 15:59 | <devsnek> | Bakkot: the spec for weakrefs adds explicit cleanup steps |
| 16:00 | <devsnek> | to weak collections |
| 16:00 | <Bakkot> | aww |
| 16:00 | <devsnek> | lol |
| 16:17 | <shu> | Bakkot: even the current spec says that |
| 16:17 | <shu> | Bakkot: it just does it in prose |
| 16:17 | <shu> | "If an object that is being used as the key of a WeakMap key/value pair is only reachable by following a chain of references that start within that WeakMap, then that key/value pair is inaccessible and is automatically removed from the WeakMap. WeakMap implementations must detect and remove such key/value pairs and any associated resources." |
| 16:18 | <shu> | whatever "remove such key/value pairs and any associated resources" means |
| 16:18 | <Bakkot> | oh, fun |
| 16:18 | <Bakkot> | but it doesn't... actually remove it |
| 16:19 | <Bakkot> | "is automatically removed from the WeakMap" is a statement, not a normative requirement, and it appears to be false |
| 16:19 | <shu> | what does actually remove it mean, like call delete? |
| 16:19 | <shu> | the reachability part is also wrong given optimizations but ehh |
| 16:19 | <Bakkot> | remove or replace its entry in map.[[WeakMapData]] |
| 16:19 | <shu> | yeah there're no operational steps to actually remove it |
| 16:20 | <shu> | however you interpret "must detect and remove", i guess |
| 16:20 | <devsnek> | weakrefs adds the actual step of setting the entry to ~empty~ |
| 16:20 | <devsnek> | I think |
| 16:21 | <bradleymeck> | devsnek: i can't minimally repro asking @jasnell who was pointing this out since he works with diagnostic tooling he should be better prepared to show an example |
| 16:22 | <bradleymeck> | repro. * |
| 16:43 | <devsnek> | bradleymeck: looks like it shouldn't have a problem, reading the source |
| 16:43 | <devsnek> | maybe the rewrite to torque fixed it but node doesn't have that yet? |
| 20:35 | <Bakkot> | devsnek: re https://github.com/engine262/engine262/blob/e4c6798e1f3f89505f4c23eecc9e5d647af6b00e/src/parse.mjs#L84-L192, it looks like you've made the chaining left-associative, which is weird |
| 20:35 | <Bakkot> | specifically, the third example |
| 20:35 | <Bakkot> | I would expect the outermost node to correspond to the first `?.`, not the second |
| 20:35 | <devsnek> | Bakkot: i tried to make it match the spec |
| 20:36 | <devsnek> | is it not doing that? |
| 20:36 | <Bakkot> | hm, let me look |
| 20:36 | <Bakkot> | I would guess no though |
| 20:39 | <Bakkot> | (actually I guess my expectation is not as strong as I thought; either representation works. but let me see what the spec's parse tree is.) |
| 20:41 | <Bakkot> | devsnek ah, you're right, you do match the spec |
| 20:41 | <devsnek> | glad to hear it 😅 |
| 20:42 | <Bakkot> | this is not how I'd've spec'd it, intuitively, but maybe there is a reason the way I'd intuitively do it doesn't work |
| 20:43 | <Bakkot> | it shouldn't make a difference semantically I think |
| 20:43 | <devsnek> | works nicely for ast evaluation |
| 20:43 | <devsnek> | which is basically what the spec does |
| 20:44 | <Bakkot> | the way I'd've done it would basically have made `?.` have the other associativity, which also works just the same for AST evaluation |
| 20:44 | <Bakkot> | and also matches my mental model of how to evaluate |
| 20:45 | <Bakkot> | basically, the way `a?.b?.c` evaluates is, you evaluate `a?.b` and then if that was non-null you evaluate `.c` |
| 20:45 | <Bakkot> | and the way I'd've done it, because it matches the way that I think about it, is that you evaluate `a` and then if that was non-null you evaluate `.b?.c` |
| 20:45 | <shu> | wait, is that not how it evaluates today? |
| 20:45 | <Bakkot> | shu which of the two things that I wrote? |
| 20:46 | <Bakkot> | the first thing I wrote is the way evaluation happens today |
| 20:46 | <shu> | the second one |
| 20:46 | <Bakkot> | nope, it's the first one |
| 20:46 | <Bakkot> | they are semantically equivalent though |
| 20:46 | <shu> | but... they commnute |
| 20:46 | <shu> | commute |
| 20:46 | <shu> | ok |
| 20:46 | <devsnek> | feel free to change it |
| 20:46 | <shu> | well, unless there's an expression stepper, i guess |
| 20:46 | <devsnek> | what is an expression stepper |
| 20:47 | <shu> | something that single-steps through expression evaluation |
| 20:47 | <devsnek> | oh |
| 20:47 | <Bakkot> | my guess is that the grammar for my preferred parse is harder to write |
| 20:47 | <Bakkot> | maybe I will think about it a little |
| 20:47 | <devsnek> | i have a stepper for engine262 |
| 20:47 | <shu> | just introduce another cover grammar, ez |
| 20:48 | <Bakkot> | :| |
| 20:49 | <shu> | MyEquivalentButEasierToReadOptionalChainCover |
| 20:49 | <rkirsling> | yeah the spec there is NOT a impl recommendation |
| 20:49 | <rkirsling> | heh |
| 20:49 | <devsnek> | hm my stepper died |
| 20:49 | <devsnek> | https://gc.gy/54170393.png |
| 20:50 | <Bakkot> | sounds like it's time for a new test! |
| 20:50 | <devsnek> | implying i have tests for this thing |
| 20:51 | <Bakkot> | it can be time for a new test even if you have zero tests |
| 20:51 | <Bakkot> | in fact that is the most frequent reason for it to be time for a new test |
| 20:51 | <shu> | well, if there's no precedent for tests, why start one now |
| 20:51 | <shu> | why rock the boat |
| 20:52 | <devsnek> | i wonder if i could capture the result of evaluation calls and print them with the stepper |
| 20:54 | <devsnek> | shu: does v8 not want js apis to have c++ apis anymore or is it just an area that needs work |
| 20:54 | <shu> | devsnek: what's the context? |
| 20:54 | <devsnek> | like how there's no v8::WeakMap |
| 20:56 | <shu> | there's no central guidance like that afaik, but there's also no effort to expose new JS features to V8 API without a need, such as for integration with the embedder |
| 20:56 | <shu> | in general, the smaller the API surface the better |
| 20:56 | <devsnek> | we do have one place in node where i would like to use v8::WeakMap::New but its not a huge deal |
| 20:56 | <devsnek> | makes sense |
| 20:56 | <shu> | namely, you don't want to look up the WeakMap property, you want to make a weakmap directly |
| 20:57 | <devsnek> | yeah there's just one area where we have to create the weakmap by traversing through the global instead of calling something directly |
| 21:01 | <shu> | i think exposing WeakMap is probably reasonable |
| 21:02 | <shu> | there isn't too much "product direction" for the API right now. may be useful to sit down with the team to decide what JS objects are reasonable to reflect in API |
| 21:05 | <bradleymeck> | v8::Context::GetIntrinsic XD, reduce all the APIs to that! (pls no) |
| 21:07 | <devsnek> | bradleymeck: if it was a const fn that might actually be okay |
| 21:45 | <rkirsling> | hey jridgewell I think this test is wrong |
| 21:45 | <rkirsling> | https://github.com/tc39/test262/blob/master/test/language/expressions/logical-assignment/lgcl-and-assignment-operator-non-simple-lhs.js |
| 21:46 | <rkirsling> | that's not an early error for += |
| 21:46 | <devsnek> | call expressions are not valid assignment targets |
| 21:46 | <devsnek> | idk why the test explicitly defines a function it isn't needed |
| 21:47 | <drousso_> | rkirsling it was actually rwaldron who wrote it :) |
| 21:47 | <devsnek> | wait are call expressions valid assignment targets |
| 21:47 | <devsnek> | is this that ie backwards compat thing |
| 21:47 | <rkirsling> | they're valid at parse time, yes |
| 21:48 | <Bakkot> | rkirsling it's an early error |
| 21:48 | <Bakkot> | but browsers all agree it should not be, for back-compat reasons |
| 21:48 | <rkirsling> | ohh |
| 21:48 | <Bakkot> | it is on my todo list to fix this discrepency |
| 21:48 | <rkirsling> | funky |
| 21:48 | <devsnek> | https://gc.gy/54173912.png |
| 21:48 | <rkirsling> | fix as in "make the spec match reality", I assume? |
| 21:48 | <Bakkot> | rkirsling yup |
| 21:48 | <rkirsling> | cool |
| 21:49 | <Bakkot> | devsnek try `false && (a() = b)` |
| 21:49 | <rkirsling> | I guess the test is valid then, we'll just intentionally fail it for now lol |
| 21:49 | <drousso_> | lol |
| 21:49 | <devsnek> | Bakkot: its the ones that throw reference errors |
| 21:49 | <Bakkot> | ah, that's something which changed kind of recently |
| 21:49 | <Bakkot> | was changed by rkirsling actually, IIRC |
| 21:49 | <rkirsling> | also yeah now that we're having this discussion I do remember the IE compatibility thing |
| 21:50 | <rkirsling> | Bakkot: yup! |
| 21:50 | <Bakkot> | https://github.com/tc39/ecma262/pull/1527 |
| 21:51 | <rkirsling> | is there an issue open for the web reality concern? |
| 21:52 | <Bakkot> | looking... |
| 21:52 | <devsnek> | are there any counters for assigning to a call expression |
| 21:52 | <Bakkot> | devsnek yes I added some to V8 ages ago |
| 21:52 | <Bakkot> | probably they are still there |
| 21:53 | <rkirsling> | yeah I'm actually surprised at myself for forgetting this whole discussion |
| 21:53 | <devsnek> | Bakkot: hm i don't see it |
| 21:53 | <Bakkot> | `lhsiscallinsloppy` or something |
| 21:53 | <devsnek> | unless it doesn't show up on https://www.chromestatus.com/metrics/feature/popularity |
| 21:53 | <devsnek> | oh nvm |
| 21:53 | <devsnek> | it just doesn't contain the word "assign" |
| 21:53 | <devsnek> | 0.06336% |
| 21:54 | <devsnek> | https://www.chromestatus.com/metrics/feature/timeline/popularity/1764 |
| 21:54 | <ljharb> | that seems high |
| 21:55 | <ljharb> | but 904 sites listed seems evangelizable, if someone wants to do the work :-) |
| 21:55 | <devsnek> | apparently people really like assigning to call expressions |
| 21:56 | <ljharb> | my guess is that 99% of the occurrences are a single library |
| 21:56 | <ljharb> | possibly 100% |
| 21:56 | <Bakkot> | ugh, I could swear there was an issue for this, but I can't find it |
| 21:56 | <ljharb> | for example, altho i can't craft a counter to prove it, i'm convinced 100% of the "non-regex objects with Symbol.match" occurrences are a core-js feature detection |
| 21:57 | <rkirsling> | oh here |
| 21:57 | <rkirsling> | https://github.com/tc39/ecma262/issues/257 |
| 21:57 | <rkirsling> | the title is wrong |
| 21:57 | <rkirsling> | but scroll to the bottom |
| 21:59 | <Bakkot> | ah fascinating |
| 22:01 | <Bakkot> | I wish I had permissions to read https://bugs.chromium.org/p/chromium/issues/detail?id=358346 |
| 22:01 | <Bakkot> | I can't see why it would not be public |
| 22:02 | <Bakkot> | anyone on chrome able to fix that ^? shu? |
| 22:02 | <Bakkot> | (this is the bug for https://codereview.chromium.org/217823003 ) |
| 22:16 | <shu> | Bakkot: that CL must be linking to the wrong bug |
| 22:17 | <shu> | Bakkot: that issue has nothing to do with JS |
| 22:17 | <Bakkot> | shu dang :( |
| 22:18 | <shu> | i tried some one-off combinations of the id and didn't land on anything useful either |
| 22:19 | <shu> | Bakkot: ah i had to read fairly down the comments to see, but that is the right bug |
| 22:19 | <shu> | i don't think i can open it however... |
| 22:20 | <Bakkot> | aw that's even worse |
| 22:20 | <Bakkot> | I need to make a note for myself that if I ever go work for google again I gotta go read that issue |
| 22:21 | <shu> | i'm pretty sure i can tell you the interesting bit though |
| 22:21 | <shu> | at the time, there was a site that broke because it had this snippet: |
| 22:21 | <shu> | x.f() += y |
| 22:22 | <devsnek> | :( |
| 22:22 | <ljharb> | any tips on who we have to email to fix that snippet? |
| 22:22 | <devsnek> | sundar⊙gc |
| 22:22 | <shu> | that i'm unsure so i'd need to check up my chain about this kind of disclosure |
| 22:23 | <shu> | also it is from 2014 |
| 22:23 | <shu> | i didn't read backlog, what's the context? |
| 22:24 | <Bakkot> | context is that `f() = y` continues to be a runtime error per browsers and a parsetime error per spec, which is bad |
| 22:24 | <shu> | ah, okay, welp |
| 22:24 | <Bakkot> | was doing archeology to see if I could find a concrete instance of why browsers need this |
| 22:25 | <shu> | it's unfortunate but i'd need to be better convinced to try this change again |
| 22:26 | <Bakkot> | I think at this point we are unlikely to get browsers to change; my intent is to make the spec match browsers |
| 22:26 | <shu> | ah |
| 22:26 | <Bakkot> | or rather, specifically, to make `f() = y` a runtime error in sloppy mode and an early error in strict mode, which matches firefox and JSC |
| 22:26 | <shu> | sgtm? |
| 22:26 | <shu> | well, in this instance it was compound assignment |
| 22:26 | <Bakkot> | yeah, compound or simple I guess |
| 22:26 | <shu> | is = standin there for all assignment operators? |
| 22:26 | <shu> | ok |
| 22:26 | <Bakkot> | there's use counters: https://github.com/tc39/ecma262/issues/257#issuecomment-502878708 |
| 22:33 | <shu> | hm those are pretty low |
| 22:35 | <shu> | strict mode seems fairly not risky |
| 22:46 | <shu> | Bakkot: i opened the issue |
| 22:48 | <Bakkot> | shu: sweet, thanks! |
| 22:50 | <shu> | (it's also a good showcase for the amount of work backward incompats cause, and why browsers often don't have any appetite for trying anything) |
