Use std::map from MSVC STL for EphemeronRememberedSet [v8/v8 : main]
Joyee Cheung (Gerrit)
Joyee Cheung voted and added 1 comment![]( "Open in Gerrit")
Votes added by Joyee Cheung
| Commit-Queue | +1 |
1 comment
Hi, as discussed in Slack I am trying to see if this patch is acceptable in the upstream to reduce the churn of V8 upgrades in Node.js. We have an issue discussing whether we can switch to libc++ on Windows but I think it will take quite some effort to make that happen https://github.com/nodejs/node/issues/58123 - so for now we'd have to float this patch in Node.js to prevent the bug in MSVC STL from crashing V8.
Related details
- Leszek Swirski
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Leszek Swirski added 1 comment![]( "Open in Gerrit")
Hi, as discussed in Slack I am trying to see if this patch is acceptable in the upstream to reduce the churn of V8 upgrades in Node.js. We have an issue discussing whether we can switch to libc++ on Windows but I think it will take quite some effort to make that happen https://github.com/nodejs/node/issues/58123 - so for now we'd have to float this patch in Node.js to prevent the bug in MSVC STL from crashing V8.
As I said on slack, normally we'd avoid this sort of thing but since this is small and localised, I'm inclined to make an exception and accept it. I'll hand over to Michael as a code owner for the final call though.
Related details
- Joyee Cheung
- Michael Lippautz
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Leszek Swirski added 1 comment![]( "Open in Gerrit")
// Work around it by using a std::map instead.please file a bug which links to the node issue, and add a TODO to remove this workaround when Node uses libc++
Related details
- Joyee Cheung
- Michael Lippautz
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joyee Cheung (Gerrit)
Joyee Cheung added 2 comments![]( "Open in Gerrit")
Hi, sorry that I haven't found the time to take care of this patch. I noticed that Node.js has been floating this patch for a couple of V8 updates, so getting back to it now. I understand that it may not be accepted in principal, but since this is small enough it might be worth an exception? It would be nice to keep it in the upstream and reduce the number of floated patches in Node.js.
please file a bug which links to the node issue, and add a TODO to remove this workaround when Node uses libc++
Opened https://issues.chromium.org/u/1/issues/469024935 and added a TODO.
Related details
- Leszek Swirski
- Michael Lippautz
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |