Skip to content

IDX-7031 Constrain cache generics with concepts#44

Open
dalloriam wants to merge 3 commits into
feature/IDX-7025-try-insertfrom
feature/stricter-concepts
Open

IDX-7031 Constrain cache generics with concepts#44
dalloriam wants to merge 3 commits into
feature/IDX-7025-try-insertfrom
feature/stricter-concepts

Conversation

@dalloriam

Copy link
Copy Markdown
Contributor

Changes

  • Move implementation of template methods inline with their declaration
    • I know this is contrary to how we usually do things, but I feel that in this case it was doing more harm than good to repeat
template<class K,
         class V,
         template<class, class, class> class I,
         template<class, class, class> class E,
         template<class, class, class> class C,
         class SV,
         class SK,
         class KH,
         LockingStrategy L>
bool Cache<K, V, I, E, C, SV, SK, KH, L>::insert_locked(K key, V value)

for every method.

  • Add new concepts to constrain cache generics

Copilot AI left a comment

Copy link
Copy Markdown

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 refactors cache/policy templates by moving template method implementations inline in headers and introduces new C++20 concepts to constrain cache generics (keys, hashers, measurement functors, etc.), aiming to provide clearer compile-time diagnostics and tighter API contracts.

Changes:

  • Moved many template method definitions (cache + policies + bloom filter utilities) inline into their class declarations.
  • Added cachemere::detail::traits concepts (e.g., Key, HasherFor, MeasureFor, etc.) and applied them to cache/policy templates.
  • Adjusted composite key hashing for basic strings.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
include/cachemere/policy/insertion_tinylfu.h Apply concepts to template params; inline method implementations.
include/cachemere/policy/insertion_always.h Apply concepts to template params; inline method implementations.
include/cachemere/policy/eviction_segmented_lru.h Apply concepts to template params; inline method implementations; rename/reshape ticket type.
include/cachemere/policy/eviction_lru.h Apply concepts to template params; inline method implementations.
include/cachemere/policy/eviction_gdsf.h Apply concepts and introduce a cost functor concept; inline method implementations.
include/cachemere/policy/detail/hash_mixer.h Constrain hasher template param; inline implementation.
include/cachemere/policy/detail/counting_bloom_filter.h Inline implementation and refactor internals (shared index computation).
include/cachemere/policy/detail/bloom_filter.h Inline implementation; adjust include ordering and add traits include.
include/cachemere/policy/constraint_memory.h Apply concepts to template params; inline method implementations.
include/cachemere/policy/constraint_count.h Apply concepts to template params; inline method implementations.
include/cachemere/detail/traits.h Add new concepts for cache generics and call-site constraints.
include/cachemere/composite_key.h Adjust string hashing to use c_str() for basic strings.
include/cachemere/cache.h Constrain cache template params with concepts; move template method implementations inline; modernize SFINAE to requires.

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

Comment thread include/cachemere/policy/eviction_gdsf.h
Comment thread include/cachemere/policy/eviction_lru.h
Comment thread include/cachemere/policy/constraint_memory.h
Comment thread include/cachemere/policy/constraint_count.h
Comment thread include/cachemere/detail/traits.h
@dalloriam
dalloriam force-pushed the feature/IDX-7025-try-insert branch from 067f16f to 8cf4125 Compare May 27, 2026 12:51
@dalloriam
dalloriam force-pushed the feature/stricter-concepts branch from 521b590 to cbb7f85 Compare May 27, 2026 12:52
@dalloriam
dalloriam marked this pull request as ready for review May 27, 2026 12:57
@dalloriam
dalloriam requested a review from bourque-julien May 28, 2026 15:12
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.

2 participants