deps: define missing operator delete functions

[jBarz]

Checklist

Affected core subsystem(s)

v8

Description of change

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.

This pull request creates definitions which if ever called, result
in an abort.

[addaleax]

I guess whether we do this or not is @nodejs/lts’s call?

[cjihrig]

I'm not a member of the LTS team, but this doesn't seem worth it for a branch that is EOL in a couple weeks.

[mscdex]

I agree with @cjihrig

[bnoordhuis]

Also, is this really an issue and not just academic?

[jBarz]

This affects the z/OS port that uses xlc compiler.
v0.12 is the first version being ported to z/OS.
I plan to upstream this change to the latest v8 so that it can be picked up by Node v6 which is our next target release on the platform.

[bnoordhuis]

Okay, I think I see. It's because those classes explicitly declare delete operators but without defining them in any compilation units, correct? Don't you also need definitions for Isolate and SealHandleScope in that case?

I should warn you that this pull request may be simply too late if you want it to show up in a release. v0.12 is so close to its end of life that it probably won't see another release again.

[jBarz]

I will cancel this PR since it is close to EOL. Will look at making these changes in node v6

It's because those classes explicitly declare delete operators but without defining them in any compilation units, correct?
Correct

Don't you also need definitions for Isolate and SealHandleScope in that case?
Yes which I missed.

[addaleax]

Will look at making these changes in node v6

If you plan on upstreaming these changes into V8 anyway, the V8 master branch might be the best way to start. Node is somewhat picky a) about the changes to deps/ that it accepts that could also be applied upstream first and b) about changes to LTS versions that would also apply on master (because there is a general rule that changes need to live 2 weeks in a Current release before going into LTS).

[bnoordhuis]

@jBarz It looks like we will be doing one more v0.12 release after all, see #10352. If you would still like to see this land, can you reopen and update the PR?

[bnoordhuis]

I've opened #10356 with the extra changes.