deps: backport 224d376 from V8 upstream#10526
Conversation
nodejs-github-bot
added
the
v8 engine
|
cc @gibfahn |
|
LGTM Did you run into this as a problem on a non-gcc system? |
|
I ran into this on z/OS which uses the xlc compiler. |
|
I think the V8 patch level needs to be updated. The process should be documented in the updating v8 guide. |
|
Weird failure in the arm CI job. Appears unrelated but just in case: https://ci.nodejs.org/job/node-test-pull-request/5730/ |
|
It is weird because when I click on the link for details, the test appears to have passed. :-) |
|
The results showing now are for the new run I just kicked off... which does appear to have passed |
|
The Github check sometimes wrongly reports that |
|
@jBarz do the changes in |
|
@gib: v8.h in the master branch in chromium underwent some changes that needed to be reversed. |
gibfahn
left a comment
gibfahn
left a comment
There was a problem hiding this comment.
In that case LGTM (in that it matches the upstream commit). Should be reviewed by @nodejs/v8 and/or @MylesBorins though.
ofrobots
left a comment
ofrobots
left a comment
There was a problem hiding this comment.
LGTM w/ nit: the commit abstract should say 'backport' as this is not a clean cherry-pick.
|
Another nit: prefer 7-character commit ids. |
Orignial commit message: Abort in delete operators that shouldn't be called. Section 3.2 of the C++ standard states that destructor definitions implicitly "use" operator delete functions. Therefore, these operator delete functions must be defined even if they are never called by user code explicitly. http://www.open-std.org/JTC1/SC22/WG21/docs/ cwg_defects.html#261 gcc allows them to remain as empty definitions. However, not all compilers allow this. (e.g. xlc on zOS). This pull request creates definitions which if ever called, result in an abort. R=danno@chromium.org,jochen@chromium.org BUG= LOG=N Review-Url: https://codereview.chromium.org/2588433002 Cr-Commit-Position: refs/heads/master@{nodejs#41981}
|
addressed nits |
jBarz
changed the title
jBarz
changed the title
|
Thanks. LGTM. |
bnoordhuis
left a comment
bnoordhuis
left a comment
There was a problem hiding this comment.
LGTM. @jBarz Did you request backports to 5.5 and 5.6?
|
oh no :-( |
|
I have requested backports to 5.5, 5.6. |
|
Does it need to be applied to v7, v6 or v4? |
|
Ping. any updates on this one? |
|
@jBarz if you could rebase and confirm this is still needed, we should get it landed, especially if it's already gone into v6.x |
|
This backport is no longer needed because it is already backported via #11752 |
9 participants
Orignial commit message:
Abort in delete operators that shouldn't be called.
Section 3.2 of the C++ standard states that destructor
definitions implicitly "use" operator delete functions.
Therefore, these operator delete functions must be
defined even if they are never called by user code
explicitly.
http://www.open-std.org/JTC1/SC22/WG21/docs/
cwg_defects.html#261
gcc allows them to remain as empty definitions. However,
not all compilers allow this. (e.g. xlc on zOS). This pull
request creates definitions which if ever called, result
in an abort.
R=danno@chromium.org,jochen@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2588433002
Cr-Commit-Position: refs/heads/master@{#41981}
Checklist
make -j4 test(UNIX)Affected core subsystem(s)
v8