src: simplify is_callable by making it a concept #58169

tniessen · 2025-05-04T17:45:35Z

Using a C++20 concept here makes is_callable much simpler than relying on SFINAE. It is equivalent for function types, std::function, lambdas, and classes with operator(), regardless of argument or return types.

Using a C++20 `concept` here makes `is_callable` much simpler than relying on SFINAE. It is equivalent for function types, `std::function`, lambdas, and classes with `operator()`, regardless of argument or return types.

anonrig · 2025-05-04T18:36:27Z

src/req_wrap-inl.h

 struct MakeLibuvRequestCallback {
  static T For(ReqWrap<ReqT>* req_wrap, T v) {
-    static_assert(!is_callable<T>::value,
+    static_assert(!is_callable<T>,


You can change type name T to is_callable T and remove this static_assertion.

codecov · 2025-05-04T18:43:20Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.14%. Comparing base (5fb879c) to head (33a9784).
⚠️ Report is 953 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #58169   +/-   ##
=======================================
  Coverage   90.13%   90.14%           
=======================================
  Files         630      630           
  Lines      186611   186611           
  Branches    36631    36634    +3     
=======================================
+ Hits       168204   168220   +16     
+ Misses      11192    11183    -9     
+ Partials     7215     7208    -7

Files with missing lines	Coverage Δ
src/req_wrap-inl.h	`92.72% <ø> (ø)`
src/util.h	`91.22% <ø> (ø)`

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jasnell · 2025-05-07T00:00:50Z

src/util.h

-    std::is_same<decltype(void(&T::operator())), void>::value
-    >::type> : std::true_type { };
+concept is_callable =
+    std::is_function<T>::value || requires { &T::operator(); };


For concepts like this, I'd prefer if we adopted a naming scheme like IsCallable

jasnell · 2025-05-07T00:01:00Z

src/req_wrap-inl.h

 // function type, it is assumed that this it is the request callback, and a
 // wrapper that calls the original callback is created.
 // If not, the parameter is passed through verbatim.
 template <typename ReqT, typename T>


Suggested change

template <typename ReqT, typename T>

template <typename ReqT, is_callable T>

jasnell · 2025-05-07T00:01:29Z

src/req_wrap-inl.h

+    static_assert(!is_callable<T>,
                  "MakeLibuvRequestCallback missed a callback");


Suggested change

static_assert(!is_callable<T>,

"MakeLibuvRequestCallback missed a callback");

nodejs-github-bot · 2025-08-14T14:58:54Z

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

nodejs-github-bot · 2025-08-23T11:56:49Z

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

nodejs-github-bot · 2025-09-12T11:18:23Z

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

nodejs-github-bot · 2025-09-19T10:44:07Z

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

nodejs-github-bot · 2025-09-20T22:44:25Z

Landed in f6d6b91

Using a C++20 `concept` here makes `is_callable` much simpler than relying on SFINAE. It is equivalent for function types, `std::function`, lambdas, and classes with `operator()`, regardless of argument or return types. PR-URL: #58169 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [node](https://nodejs.org) ([source](https://github.com/nodejs/node)) | minor | `24.8.0` -> `24.9.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>nodejs/node (node)</summary> ### [`v24.9.0`](https://github.com/nodejs/node/releases/tag/v24.9.0): 2025-09-25, Version 24.9.0 (Current), @targos [Compare Source](nodejs/node@v24.8.0...v24.9.0) ##### Notable Changes - \[[`9b043a9096`](nodejs/node@9b043a9096)] - **(SEMVER-MINOR)** **http**: add shouldUpgradeCallback to let servers control HTTP upgrades (Tim Perry) [#59824](nodejs/node#59824) - \[[`a6456ab90a`](nodejs/node@a6456ab90a)] - **(SEMVER-MINOR)** **sqlite**: cleanup ERM support and export Session class (James M Snell) [#58378](nodejs/node#58378) - \[[`5563361d22`](nodejs/node@5563361d22)] - **(SEMVER-MINOR)** **sqlite**: add tagged template (0hm☘️) [#58748](nodejs/node#58748) - \[[`04013ee933`](nodejs/node@04013ee933)] - **(SEMVER-MINOR)** **worker**: add heap profile API (theanarkh) [#59846](nodejs/node#59846) ##### Commits - \[[`cbec4fd6de`](nodejs/node@cbec4fd6de)] - **benchmark**: calibrate config dgram multi-buffer (Bruno Rodrigues) [#59696](nodejs/node#59696) - \[[`9a4bbdc3c5`](nodejs/node@9a4bbdc3c5)] - **benchmark**: calibrate config cluster/echo.js (Nam Yooseong) [#59836](nodejs/node#59836) - \[[`0b284d86e8`](nodejs/node@0b284d86e8)] - **build**: add the missing macro definitions for OpenHarmony (hqzing) [#59804](nodejs/node#59804) - \[[`43e6e54d66`](nodejs/node@43e6e54d66)] - **build**: do not include custom ESLint rules testing in tarball (Antoine du Hamel) [#59809](nodejs/node#59809) - \[[`039ac19154`](nodejs/node@039ac19154)] - **crypto**: expose signatureAlgorithm on X509Certificate (Patrick Costa) [#59235](nodejs/node#59235) - \[[`647c332704`](nodejs/node@647c332704)] - **crypto**: use `return await` when returning Promises from async functions (Renegade334) [#59841](nodejs/node#59841) - \[[`8ed4587cf0`](nodejs/node@8ed4587cf0)] - **crypto**: use async functions for non-stub Promise-returning functions (Renegade334) [#59841](nodejs/node#59841) - \[[`bb051c56ef`](nodejs/node@bb051c56ef)] - **crypto**: avoid calls to `promise.catch()` (Renegade334) [#59841](nodejs/node#59841) - \[[`05e560dd25`](nodejs/node@05e560dd25)] - **deps**: update googletest to [`50b8600`](nodejs/node@50b8600) (Node.js GitHub Bot) [#59955](nodejs/node#59955) - \[[`fa40d3a785`](nodejs/node@fa40d3a785)] - **deps**: update archs files for openssl-3.5.3 (Node.js GitHub Bot) [#59901](nodejs/node#59901) - \[[`8c85570d18`](nodejs/node@8c85570d18)] - **deps**: upgrade openssl sources to openssl-3.5.3 (Node.js GitHub Bot) [#59901](nodejs/node#59901) - \[[`b71125664e`](nodejs/node@b71125664e)] - **deps**: update undici to 7.16.0 (Node.js GitHub Bot) [#59830](nodejs/node#59830) - \[[`dea5dd7077`](nodejs/node@dea5dd7077)] - **dgram**: restore buffer optimization in fixBufferList (Yoo) [#59934](nodejs/node#59934) - \[[`b0c1e67532`](nodejs/node@b0c1e67532)] - **diagnostics\_channel**: fix race condition with diagnostics\_channel and GC (Ugaitz Urien) [#59910](nodejs/node#59910) - \[[`0b37b594c3`](nodejs/node@0b37b594c3)] - **doc**: use "WebAssembly" instead of "Web Assembly" (Tobias Nießen) [#59954](nodejs/node#59954) - \[[`1e723f9c6b`](nodejs/node@1e723f9c6b)] - **doc**: fix typo in section on microtask order (Tobias Nießen) [#59932](nodejs/node#59932) - \[[`a28962a85c`](nodejs/node@a28962a85c)] - **doc**: update V8 fast API guidance (René) [#58999](nodejs/node#58999) - \[[`bd767c5d1b`](nodejs/node@bd767c5d1b)] - **doc**: add security escalation policy (Ulises Gascón) [#59806](nodejs/node#59806) - \[[`9df91e59e1`](nodejs/node@9df91e59e1)] - **doc**: type improvement of file `http.md` (yusheng chen) [#58189](nodejs/node#58189) - \[[`e4f571680b`](nodejs/node@e4f571680b)] - **doc**: deprecate closing `fs.Dir` on garbage collection (Livia Medeiros) [#59839](nodejs/node#59839) - \[[`e9cb986fa5`](nodejs/node@e9cb986fa5)] - **doc**: rephrase dynamic import() description (Nam Yooseong) [#59224](nodejs/node#59224) - \[[`026d4e33f7`](nodejs/node@026d4e33f7)] - **doc,crypto**: update subtle.generateKey and subtle.importKey (Filip Skokan) [#59851](nodejs/node#59851) - \[[`2b2591db52`](nodejs/node@2b2591db52)] - **esm**: make hasAsyncGraph non-enumerable (Joyee Cheung) [#59905](nodejs/node#59905) - \[[`993f05d323`](nodejs/node@993f05d323)] - **fs,win**: do not add a second trailing slash in readdir (Gerhard Stöbich) [#59847](nodejs/node#59847) - \[[`7aec53b607`](nodejs/node@7aec53b607)] - **(SEMVER-MINOR)** **http**: add shouldUpgradeCallback to let servers control HTTP upgrades (Tim Perry) [#59824](nodejs/node#59824) - \[[`83ae6102e7`](nodejs/node@83ae6102e7)] - **http**: optimize checkIsHttpToken for short strings (방진혁) [#59832](nodejs/node#59832) - \[[`6695067636`](nodejs/node@6695067636)] - **http,https**: handle IPv6 with proxies (Joyee Cheung) [#59894](nodejs/node#59894) - \[[`c5d910a0a9`](nodejs/node@c5d910a0a9)] - **http2**: fix allowHttp1+Upgrade, broken by shouldUpgradeCallback (Tim Perry) [#59924](nodejs/node#59924) - \[[`acada1fb82`](nodejs/node@acada1fb82)] - **inspector**: ensure adequate memory allocation for `Binary::toBase64` (René) [#59870](nodejs/node#59870) - \[[`396cc8ec65`](nodejs/node@396cc8ec65)] - **lib**: update inspect output format for subclasses (Miguel Marcondes Filho) [#59687](nodejs/node#59687) - \[[`fed1dac8de`](nodejs/node@fed1dac8de)] - **lib**: update isDeepStrictEqual to support options (Miguel Marcondes Filho) [#59762](nodejs/node#59762) - \[[`d785929fd7`](nodejs/node@d785929fd7)] - **lib**: add source map support for assert messages (Chengzhong Wu) [#59751](nodejs/node#59751) - \[[`ff13d1d61e`](nodejs/node@ff13d1d61e)] - **lib,src**: cache ModuleWrap.hasAsyncGraph (Chengzhong Wu) [#59703](nodejs/node#59703) - \[[`b200cd8470`](nodejs/node@b200cd8470)] - **lib,src**: refactor assert to load error source from memory (Chengzhong Wu) [#59751](nodejs/node#59751) - \[[`e94c57301b`](nodejs/node@e94c57301b)] - **meta**: add .npmrc with ignore-scripts=true (Joyee Cheung) [#59914](nodejs/node#59914) - \[[`728472a57b`](nodejs/node@728472a57b)] - **module**: only put directly require-d ESM into require.cache (Joyee Cheung) [#59874](nodejs/node#59874) - \[[`be48760b93`](nodejs/node@be48760b93)] - **node-api**: added SharedArrayBuffer api (Mert Can Altin) [#59071](nodejs/node#59071) - \[[`f006a14522`](nodejs/node@f006a14522)] - **node-api**: make napi\_delete\_reference use node\_api\_basic\_env (Jeetu Suthar) [#59684](nodejs/node#59684) - \[[`0f46c1c3b0`](nodejs/node@0f46c1c3b0)] - **repl**: fix cpu overhead pasting big strings to the REPL (Ruben Bridgewater) [#59857](nodejs/node#59857) - \[[`3eeb7b47ea`](nodejs/node@3eeb7b47ea)] - **sqlite**: fix crash session extension callbacks with workers (Bart Louwers) [#59848](nodejs/node#59848) - \[[`0fe53375ec`](nodejs/node@0fe53375ec)] - **(SEMVER-MINOR)** **sqlite**: cleanup ERM support and export Session class (James M Snell) [#58378](nodejs/node#58378) - \[[`9a3e58a007`](nodejs/node@9a3e58a007)] - **(SEMVER-MINOR)** **sqlite**: add tagged template (0hm☘️) [#58748](nodejs/node#58748) - \[[`f14ed5ab7b`](nodejs/node@f14ed5ab7b)] - **src**: simplify watchdog instantiations via `std::optional` (Anna Henningsen) [#59960](nodejs/node#59960) - \[[`e330f03f84`](nodejs/node@e330f03f84)] - **src**: update crypto objects to use DictionaryTemplate (James M Snell) [#59942](nodejs/node#59942) - \[[`69b5607cf4`](nodejs/node@69b5607cf4)] - **src**: simplify is\_callable by making it a concept (Tobias Nießen) [#58169](nodejs/node#58169) - \[[`86150f3401`](nodejs/node@86150f3401)] - **src**: rename private fields to follow naming convention (Moonki Choi) [#59923](nodejs/node#59923) - \[[`d17f299539`](nodejs/node@d17f299539)] - **src**: use DictionaryTemplate more in URLPattern (James M Snell) [#59892](nodejs/node#59892) - \[[`ac784912ac`](nodejs/node@ac784912ac)] - **src**: reduce the nearest parent package JSON cache size (Michael Smith) [#59888](nodejs/node#59888) - \[[`abecdcb536`](nodejs/node@abecdcb536)] - **src**: replace FIXED\_ONE\_BYTE\_STRING with Environment-cached strings (Moonki Choi) [#59891](nodejs/node#59891) - \[[`2bb152500b`](nodejs/node@2bb152500b)] - **src**: create strings in `FIXED_ONE_BYTE_STRING` as internalized (Anna Henningsen) [#59826](nodejs/node#59826) - \[[`03116a7cd8`](nodejs/node@03116a7cd8)] - **src**: remove `std::array` overload of `FIXED_ONE_BYTE_STRING` (Anna Henningsen) [#59826](nodejs/node#59826) - \[[`8a5325d6e3`](nodejs/node@8a5325d6e3)] - **src**: ensure `v8::Eternal` is empty before setting it (Anna Henningsen) [#59825](nodejs/node#59825) - \[[`f0c20ccd81`](nodejs/node@f0c20ccd81)] - **src**: remove unnecessary `Environment::GetCurrent()` calls (Moonki Choi) [#59814](nodejs/node#59814) - \[[`213188e491`](nodejs/node@213188e491)] - **stream**: use new AsyncResource instead of bind (Matteo Collina) [#59867](nodejs/node#59867) - \[[`ce8435b003`](nodejs/node@ce8435b003)] - **test**: testcase demonstrating issue 59541 (Eric Rannaud) [#59801](nodejs/node#59801) - \[[`8f32746142`](nodejs/node@8f32746142)] - **test**: guard write to proxy client if proxy connection is ended (Joyee Cheung) [#59742](nodejs/node#59742) - \[[`6790093fcb`](nodejs/node@6790093fcb)] - **tls**: load bundled and extra certificates off-thread (Joyee Cheung) [#59856](nodejs/node#59856) - \[[`f5d3f919d8`](nodejs/node@f5d3f919d8)] - **tls**: only do off-thread certificate loading on loading tls (Joyee Cheung) [#59856](nodejs/node#59856) - \[[`87bbaa23a0`](nodejs/node@87bbaa23a0)] - **tools**: fix `tools/make-v8.sh` for clang (Richard Lau) [#59893](nodejs/node#59893) - \[[`0d23fd525b`](nodejs/node@0d23fd525b)] - **tools**: skip test-internet workflow for draft MRs (Michaël Zasso) [#59817](nodejs/node#59817) - \[[`e17c73731a`](nodejs/node@e17c73731a)] - **tools**: copyedit `build-tarball.yml` (Antoine du Hamel) [#59808](nodejs/node#59808) - \[[`97c4e1bac9`](nodejs/node@97c4e1bac9)] - **typings**: remove unused imports (Nam Yooseong) [#59880](nodejs/node#59880) - \[[`8b29bbca76`](nodejs/node@8b29bbca76)] - **url**: replaced slice with at (Mikhail) [#59181](nodejs/node#59181) - \[[`6458867a6b`](nodejs/node@6458867a6b)] - **url**: add type checking to urlToHttpOptions() (simon-id) [#59753](nodejs/node#59753) - \[[`3c62b3886f`](nodejs/node@3c62b3886f)] - **util**: inspect objects with throwing Symbol.toStringTag (Ruben Bridgewater) [#59860](nodejs/node#59860) - \[[`6133a82875`](nodejs/node@6133a82875)] - **util**: fix debuglog.enabled not being present with callback logger (Ruben Bridgewater) [#59858](nodejs/node#59858) - \[[`9347ddddf4`](nodejs/node@9347ddddf4)] - **vm**: explain how to share promises between contexts w/ afterEvaluate (Eric Rannaud) [#59801](nodejs/node#59801) - \[[`44ce971619`](nodejs/node@44ce971619)] - **vm**: "afterEvaluate", evaluate() return a promise from the outer context (Eric Rannaud) [#59801](nodejs/node#59801) - \[[`6e586a1409`](nodejs/node@6e586a1409)] - **vm**: expose hasTopLevelAwait on SourceTextModule (Chengzhong Wu) [#59865](nodejs/node#59865) - \[[`49747a58a3`](nodejs/node@49747a58a3)] - **(SEMVER-MINOR)** **worker**: add heap profile API (theanarkh) [#59846](nodejs/node#59846) - \[[`b970c0bbc2`](nodejs/node@b970c0bbc2)] - **zlib**: reduce code duplication (jhofstee) [#57810](nodejs/node#57810) - \[[`9782ca2b1b`](nodejs/node@9782ca2b1b)] - **zlib**: implement fast path for crc32 (Gürgün Dayıoğlu) [#59813](nodejs/node#59813) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Using a C++20 `concept` here makes `is_callable` much simpler than relying on SFINAE. It is equivalent for function types, `std::function`, lambdas, and classes with `operator()`, regardless of argument or return types. PR-URL: #58169 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

tniessen added the c++ Issues and PRs that require attention from people who are familiar with C++. label May 4, 2025

nodejs-github-bot added the needs-ci PRs that need a full CI run. label May 4, 2025

src: simplify is_callable by making it a concept

33a9784

Using a C++20 `concept` here makes `is_callable` much simpler than relying on SFINAE. It is equivalent for function types, `std::function`, lambdas, and classes with `operator()`, regardless of argument or return types.

tniessen force-pushed the simplify-is-callable branch from cf9d612 to 33a9784 Compare May 4, 2025 17:47

anonrig reviewed May 4, 2025

View reviewed changes

jasnell reviewed May 7, 2025

View reviewed changes

anonrig approved these changes Jun 26, 2025

View reviewed changes

tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2025

tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 20, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 20, 2025

nodejs-github-bot merged commit f6d6b91 into nodejs:main Sep 20, 2025
61 checks passed

github-actions bot mentioned this pull request Sep 24, 2025

2025-09-25, Version 24.9.0 (Current) #59997

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

src: simplify is_callable by making it a concept #58169

src: simplify is_callable by making it a concept #58169

Uh oh!

tniessen commented May 4, 2025

Uh oh!

anonrig May 4, 2025

Uh oh!

codecov bot commented May 4, 2025 •

edited

Loading

Uh oh!

jasnell May 7, 2025

Uh oh!

jasnell May 7, 2025

Uh oh!

jasnell May 7, 2025

Uh oh!

nodejs-github-bot commented Aug 14, 2025

Uh oh!

nodejs-github-bot commented Aug 23, 2025

Uh oh!

nodejs-github-bot commented Sep 12, 2025

Uh oh!

nodejs-github-bot commented Sep 19, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 20, 2025

Uh oh!

Uh oh!

	template <typename ReqT, typename T>
	template <typename ReqT, is_callable T>

		static_assert(!is_callable<T>,
		"MakeLibuvRequestCallback missed a callback");

Uh oh!

src: simplify is_callable by making it a concept #58169

src: simplify is_callable by making it a concept #58169

Uh oh!

Conversation

tniessen commented May 4, 2025

Uh oh!

anonrig May 4, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jasnell May 7, 2025

Choose a reason for hiding this comment

Uh oh!

jasnell May 7, 2025

Choose a reason for hiding this comment

Uh oh!

jasnell May 7, 2025

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Aug 14, 2025

Uh oh!

nodejs-github-bot commented Aug 23, 2025

Uh oh!

nodejs-github-bot commented Sep 12, 2025

Uh oh!

nodejs-github-bot commented Sep 19, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 20, 2025

Uh oh!

Uh oh!

codecov bot commented May 4, 2025 •

edited

Loading