remove unused catch bindings

[cjihrig]

NOTE: I didn't update the unused bindings in the stream subsystem, as I assume that would break readable-stream.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[nodejs-github-bot]

@cjihrig build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1486/pipeline

[sam-github]

It would also be handy to have eslint check for unused catch bindings, so they don't creep back into the code.

[cjihrig]

@sam-github enabled linting for unused catch bindings. Good idea.

[refack]

I'm +1 on this (both for readability, and explicit-y), but there was the "backporting" argument made in another PR.

@bmeurer does not binding the unused exception object provide any performance benefit (or will in the future 🤞 )?

It would also be handy to have eslint check for unused catch bindings, so they don't creep back into the code.

I would actually suggest the opposite, lint for these and force the author explicitly comment why it's Ok (out of scope of this PR).

[bmeurer]

@refack V8 doesn't need to allocate the catch context in that case, which makes the exception object available. TurboFan will probably optimize that away anyways, but for bytecode it's a nice saving.

[cjihrig]

CI: https://ci.nodejs.org/job/node-test-pull-request/18366/

EDIT: Green CI: https://ci.nodejs.org/job/node-test-pull-request/18368/

[Jimbly]

This PR appears to be crashing Travis CI on PRs since this was merged? https://travis-ci.com/nodejs/node/builds/90427486

$ make lint make[1]: Entering directory `/home/travis/build/nodejs/node'Running JS linter.../home/travis/build/nodejs/node/.eslintrc.js:20 } catch{ ^SyntaxError: Unexpected token{ at createScript (vm.js:80:10) at Object.runInThisContext (vm.js:139:10) at Module._compile (module.js:599:28) at Object.Module._extensions..js (module.js:646:10) at Module.load (module.js:554:32) at tryModuleLoad (module.js:497:12) at Function.Module._load (module.js:489:3) at Module.require (module.js:579:17) at require (internal/module.js:11:18) at module.exports (/home/travis/build/nodejs/node/tools/node_modules/eslint/node_modules/require-uncached/index.js:28:9)

[codebytere]

@cjihrig i see don't-lands for v6.x and v8.x; should this also not be backported to v10.x?

[cjihrig]

I believe optional catch bindings are supported in Node 10, so it can be backported.