LadybirdBrowser / ladybird

Truly independent web browser
https://ladybird.org
BSD 2-Clause "Simplified" License
21.81k stars 966 forks source link

Remove FinalizationRegistry.prototype.cleanupSome() #2228

Open linusg opened 5 hours ago

linusg commented 5 hours ago

The proposal has been withdrawn, related test262 tests were deleted, and it's no longer mentioned on MDN.

$262.clearKeptObjects() only existed for this purpose and is no longer needed (it never was documented in test262's INTERPRETING.md to begin with).

ADKaster commented 5 hours ago

In https://github.com/tc39/notes/blob/cb328656919c1d6b658099c41bc13df622f649a9/meetings/2023-09/september-28.md#withdrawing-finalization-registry-cleanupsome one of the comments was like so:

SYG: In terms of the code being there, we are not removing anything from implementations. It’s needed for internal implementation and now needed to be exposed by a harness hook for test 262.

DE: Sorry. Just a comment. I didn’t realize that’s how V8 is, but sure.

This makes it sound like v8's implementation still needs a manual cleanup hook in order to verify the constraints of test262?

And in test262#4239, ptomato mentioned

Non-deterministic tests are not that useful anyway; these tests are probably better left to implementations' internal testing strategies.

--

Given that, does the LibJS implementation still need to keep the internal APIs for a manual, sync cleanup and/or expose a new one for test-js tests?

linusg commented 5 hours ago

This makes it sound like v8's implementation still needs a manual cleanup hook in order to verify the constraints of test262?

I think what Shu meant is that the general mechanism already existed internally before the proposal (the same is true in LibJS, clearKeptObjects() simply calls VM::finish_execution_generation()) and nothing about that changes. It needed to be exposed for test262 at the time but those tests are gone now.

Given that, does the LibJS implementation still need to keep the internal APIs for a manual, sync cleanup

Yes, VM::finish_execution_generation() also gets called at the end of JS execution.

and/or expose a new one for test-js tests?

The current FinalizationRegistry tests look a bit lacking so I guess that could be done, yeah.