v8: fix debug builds on Windows #13634
Conversation
Adds missing return which fixes debug builds on Windows Fixes: nodejs#13392
nodejs-github-bot
added
v8 engine
|
cc/ @nodejs/v8 |
|
Is the master branch also affected? I so this should target it. I will then include the commit in #13631 |
That makes sense, I think we should try to avoid v8.x/master differences where possible. |
No, this came from c8be718 (which contains parts of v8/v8@18a26cfe174) as parts of the ABI compatibility patches. The breaking code is in V8 6.0, so we may need to re-apply it once we upgrade, but ideally we could do that over upstream merge requests. This should be safe to fast-track, it’s just adding an unused return value. |
Adds missing return which fixes debug builds on Windows Fixes: #13392 Ref: https://codereview.chromium.org/2929993003/ PR-URL: #13634 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Confirmed (empirically). |
|
Comment by @hammacher on the upstream patch:
Can somebody here check that? It seems reasonable to me but I’m not a Windows person. |
|
I'll try to compile with that patch and get back to you. |
|
Yep, that CL also fixes the issue |
|
Just FYI issue does not happen on VS2017 |
|
Adds missing return which fixes debug builds on Windows Fixes: #13392 Ref: https://codereview.chromium.org/2929993003/ PR-URL: #13634 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Adds missing return which fixes debug builds on Windows Fixes: #13392 Ref: https://codereview.chromium.org/2929993003/ PR-URL: #13634 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Adds missing return which fixes debug builds on Windows Fixes: #13392 Ref: https://codereview.chromium.org/2929993003/ PR-URL: #13634 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Adds missing return which fixes debug builds on Windows Fixes: #13392 Ref: https://codereview.chromium.org/2929993003/ PR-URL: #13634 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Adds missing return which fixes debug builds on Windows Fixes: #13392 Ref: https://codereview.chromium.org/2929993003/ PR-URL: #13634 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Adds missing return which fixes debug builds on Windows Fixes: #13392 Ref: https://codereview.chromium.org/2929993003/ PR-URL: #13634 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Adds missing return which fixes debug builds on Windows Fixes: nodejs#13392 Ref: https://codereview.chromium.org/2929993003/ PR-URL: nodejs#13634 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Adds missing return which fixes debug builds on Windows Fixes: #13392 Ref: https://codereview.chromium.org/2929993003/ PR-URL: #13634 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Adds missing return which fixes debug builds on Windows Fixes: nodejs#13392 Ref: https://codereview.chromium.org/2929993003/ Refs: nodejs#13634 PR-URL: nodejs#14582 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Adds missing return which fixes Node 8.x debug builds on Windows, while we wait for upstream patch to land.
Fixes: #13392
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
v8