``` let foo = null; foo ??= function() {}; console.log(foo.name); // should be "foo", but is currently "" ``` which matches the functionality of regular assignments ``` let foo = function() {}; console.log(foo.name); // "foo" ``` Issue: <https://github.com/tc39/proposal-logical-assignment/issues/23> PR: <https://github.com/tc39/proposal-logical-assignment/pull/24>
Created attachment 400884 [details] Patch
Comment on attachment 400884 [details] Patch r=me as long as the test262 update is incoming.
Nice. Yeah there should be some test associated with this!
I manually tested the test262 updates in <https://github.com/tc39/test262/pull/2649> and we passed them all :) @yusukesuzuki suggested that I land this rather than wait for the tests so we can get additional fuzzing coverage. I'm inclined to agree, especially with how simple this change is and the fact that we already pass the tests. Thoughts?
(In reply to Devin Rousso from comment #4) > I manually tested the test262 updates in > <https://github.com/tc39/test262/pull/2649> and we passed them all :) > > @yusukesuzuki suggested that I land this rather than wait for the tests so > we can get additional fuzzing coverage. > > I'm inclined to agree, especially with how simple this change is and the > fact that we already pass the tests. > > Thoughts? Why not just write your own test?
(In reply to Saam Barati from comment #5) > (In reply to Devin Rousso from comment #4) > > I manually tested the test262 updates in <https://github.com/tc39/test262/pull/2649> and we passed them all :) > > > > @yusukesuzuki suggested that I land this rather than wait for the tests so we can get additional fuzzing coverage. > > > > I'm inclined to agree, especially with how simple this change is and the fact that we already pass the tests. > > > > Thoughts? > > Why not just write your own test? I could, but I would write basically the exact same tests, so I thought it was duplicate effort for no reward (same code being tested twice).
(In reply to Devin Rousso from comment #6) > (In reply to Saam Barati from comment #5) > > (In reply to Devin Rousso from comment #4) > > > I manually tested the test262 updates in <https://github.com/tc39/test262/pull/2649> and we passed them all :) > > > > > > @yusukesuzuki suggested that I land this rather than wait for the tests so we can get additional fuzzing coverage. > > > > > > I'm inclined to agree, especially with how simple this change is and the fact that we already pass the tests. > > > > > > Thoughts? > > > > Why not just write your own test? > > I could, but I would write basically the exact same tests, so I thought it > was duplicate effort for no reward (same code being tested twice). it's helpful to have tests that live outside test262
(In reply to Devin Rousso from comment #6) > (In reply to Saam Barati from comment #5) > > (In reply to Devin Rousso from comment #4) > > > I manually tested the test262 updates in <https://github.com/tc39/test262/pull/2649> and we passed them all :) > > > > > > @yusukesuzuki suggested that I land this rather than wait for the tests so we can get additional fuzzing coverage. > > > > > > I'm inclined to agree, especially with how simple this change is and the fact that we already pass the tests. > > > > > > Thoughts? > > > > Why not just write your own test? > > I could, but I would write basically the exact same tests, so I thought it > was duplicate effort for no reward (same code being tested twice). I like adding these tests into JSTests/stress/ even if test262 has duplicate ones. test262 is shared with other engines. Due to some mistakes in test262, the coverage in JSC can be regressed. One example is this[1]. test262 putted BigInt to utility file and every tests using this utility file failed in JSC even if the test is not using BigInt. I'm seeing very similar issues in WPT, and I think dedicated JSC stress tests is better in terms of regression testing. [1]: https://github.com/tc39/test262/issues/1252
Created attachment 401174 [details] Patch Thanks for the feedback @saamyjoon and @yusukesuzuki, I had not considered test262 itself having issues. I've added some test cases to JSTests/stress/logical-assignment-{aoperator-nd,coalesce,or}.js :)
Committed r262638: <https://trac.webkit.org/changeset/262638> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401174 [details].
<rdar://problem/64039177>