Skip to content

refactor: replace remaining std::enable_if patterns with C++20 concepts#7645

Merged
DennisOSRM merged 2 commits into
masterfrom
sfinae-to-concepts-remaining
Jul 4, 2026
Merged

refactor: replace remaining std::enable_if patterns with C++20 concepts#7645
DennisOSRM merged 2 commits into
masterfrom
sfinae-to-concepts-remaining

Conversation

@DennisOSRM

@DennisOSRM DennisOSRM commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Completes the SFINAE → C++20 concepts migration started in #7529 by replacing the last 6 std::enable_if patterns across 4 files.

Changes

File Old (SFINAE) New (concepts)
include/util/integer_range.hpp enable_if<is_integral<Integer>> requires std::integral<Integer>
include/util/json_deep_compare.hpp enable_if<!is_same<T1,T2>> requires(!std::same_as<T1,T2>)
include/guidance/turn_data_container.hpp enable_if<Ownership==Container> (×2) requires(Ownership != View)
include/server/api/parameters_parser.hpp enable_if<is_parameter_t<T>> (×2) requires is_parameter_t<T>::value

Notable: turn_data_container.hpp fix

The original enable_if was missing ::type (std::enable_if<cond> instead of std::enable_if<cond>::type), making it a no-op — push_back() and append() were always available regardless of Ownership. Both Container and External map to std::vector internally (only View maps to non-mutable vector_view), so the correct constraint is Ownership != View.

Verification

  • ✅ Build: 117/117 targets compile with C++20
  • ✅ Tests: 29/29 pass (100%)
  • ✅ CI: 21/21 checks green (clang 18/19/20, gcc 14/15, macOS/Windows/Linux)
  • ✅ Zero enable_if/void_t/is_detected patterns remain in OSRM source code

Closes #7530

🤖 Claude Code, deepseek-v4-pro

…ts/requires

Replace all 6 remaining std::enable_if SFINAE patterns across 4 files
with C++20 requires-clauses using standard concepts:

- include/util/integer_range.hpp: enable_if<is_integral> -> requires
  std::integral<Integer>; replace <type_traits> with <concepts>
- include/util/json_deep_compare.hpp: enable_if<!is_same> -> requires
  (!std::same_as<T1, T2>); add <concepts> header
- include/guidance/turn_data_container.hpp: enable_if<Ownership==Container>
  -> requires(Ownership != View); the original enable_if was missing
  ::type (non-functional), actual behavior constrains out only View
  since both Container and External map to std::vector internally
- include/server/api/parameters_parser.hpp: enable_if<is_parameter_t>
  -> requires is_parameter_t<ParameterT>::value

Build: 117/117 targets compile, all 29 tests pass.
Refs: #7530
Copilot AI review requested due to automatic review settings July 4, 2026 14:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the repository’s remaining targeted SFINAE (std::enable_if) cleanups by replacing the last few patterns with C++20 requires-clauses / standard concepts, making template constraints explicit and improving diagnostics.

Changes:

  • Replaced std::enable_if-based constraints with C++20 concepts/requires in utility and API headers.
  • Updated TurnDataContainerImpl mutating methods to be properly constrained via requires(Ownership != View) (fixing the prior ineffective enable_if usage).
  • Simplified constrained templates by removing dummy enable_if parameters and using direct constraint expressions.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
include/util/integer_range.hpp Constrains irange() to std::integral via requires and drops enable_if usage.
include/util/json_deep_compare.hpp Uses requires(!std::same_as<T1, T2>) for the cross-type comparator overload.
include/guidance/turn_data_container.hpp Correctly constrains push_back()/append() to non-View ownership using requires.
include/server/api/parameters_parser.hpp Replaces enable_if template parameters with requires constraints for parseParameters.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread include/util/integer_range.hpp Outdated
Comment on lines +4 to +5
#include <iterator>
#include <type_traits>
#include <concepts>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the follow-up formatting commit (1cce17647) — <concepts> is now before <iterator>.

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.80%. Comparing base (e3b8893) to head (85bf69d).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7645      +/-   ##
==========================================
- Coverage   94.20%   90.80%   -3.41%     
==========================================
  Files         483      483              
  Lines       37670    37670              
==========================================
- Hits        35487    34205    -1282     
- Misses       2183     3465    +1282     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

@DennisOSRM DennisOSRM force-pushed the sfinae-to-concepts-remaining branch from 1cce176 to 85bf69d Compare July 4, 2026 15:44
@DennisOSRM DennisOSRM merged commit 58a9cbf into master Jul 4, 2026
21 of 22 checks passed
@DennisOSRM DennisOSRM deleted the sfinae-to-concepts-remaining branch July 4, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SFINAE to Concepts migration

2 participants