worker: leave TODO comments for using std::variant when possible #38788

addaleax · 2021-05-23T15:23:07Z

We don’t use C++17 as a baseline yet, but once we do, we can use
std::variant to make a few things more performant (and cleaner).
There is a similar TODO comment in the crypto source already.

We don’t use C++17 as a baseline yet, but once we do, we can use `std::variant` to make a few things more performant (and cleaner). There is a similar TODO comment in the crypto source already.

github-actions · 2021-05-23T15:23:32Z

Fast-track has been requested by @addaleax. Please 👍 to approve.

nodejs-github-bot · 2021-05-23T15:23:47Z

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

We don’t use C++17 as a baseline yet, but once we do, we can use `std::variant` to make a few things more performant (and cleaner). There is a similar TODO comment in the crypto source already. PR-URL: #38788 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

addaleax · 2021-05-24T12:53:56Z

Landed in 28e1b2e

richardlau · 2021-05-24T13:02:37Z

We don’t use C++17 as a baseline yet, but once we do,

Any thoughts on when we might? We raised the minimum gcc/g++ to 8.3 in #37871 -- would we need another minimum version bump?

addaleax · 2021-05-24T13:09:54Z

@richardlau I don’t know, I think historically we’ve basically followed along with V8’s minimums. I’m personally fine with saying we’re using C++17 everywhere.

jasnell · 2021-05-24T20:17:48Z

We raised the minimum gcc/g++ to 8.3 in #37871 -- would we need another minimum version bump?

I don't believe so. As far as I'm aware, gcc/g++ 8.3 has full support for C++17. I wouldn't mind bumping up to it now.

richardlau · 2021-05-25T10:51:20Z

Opened #38807 to bump to C++17.

We don’t use C++17 as a baseline yet, but once we do, we can use `std::variant` to make a few things more performant (and cleaner). There is a similar TODO comment in the crypto source already. PR-URL: #38788 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

richardlau · 2021-07-16T13:40:25Z

Marking as dont-land-on-v14x as it's a TODO that would never be done on v14.x as we'd not bump the minimum C++ standard during a release line.

Enables C++17 features. PR-URL: #38807 Refs: #38788 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ash Cripps <acripps@redhat.com>

PR-URL: #38807 Refs: #38788 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ash Cripps <acripps@redhat.com>

PR-URL: nodejs#35415 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> deps: patch for v8 on windows PR-URL: nodejs#40010 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> deps: patch v8 for vs2019 in std17 PR-URL: nodejs#40060 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> deps: workaround debug link error on Windows PR-URL: nodejs#38807 Refs: nodejs#38788 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ash Cripps <acripps@redhat.com>

PR-URL: #35415 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> deps: patch for v8 on windows PR-URL: #40010 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> deps: patch v8 for vs2019 in std17 PR-URL: #40060 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> deps: workaround debug link error on Windows PR-URL: #38807 Refs: #38788 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ash Cripps <acripps@redhat.com>

PR-URL: nodejs/node#35415 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> deps: patch for v8 on windows PR-URL: nodejs/node#40010 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> deps: patch v8 for vs2019 in std17 PR-URL: nodejs/node#40060 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> deps: workaround debug link error on Windows PR-URL: nodejs/node#38807 Refs: nodejs/node#38788 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ash Cripps <acripps@redhat.com>