Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENHANCEMENT]: Provide helper function cuco::experimental::make_static_set et al #370

Open
esoha-nvidia opened this issue Sep 21, 2023 · 6 comments
Labels
P2: Nice to have Desired, but not necessary type: improvement Improvement / enhancement to an existing function

Comments

@esoha-nvidia
Copy link

Is your feature request related to a problem? Please describe.

I would like a function called make_static_set that will create a static set that will infer the template arguments from the function call.

Describe the solution you'd like

Instead of:

auto set = cuco::experimental::static_set<MyKeyType, MyExtent, MyThreadScope, MyKeyEqual>(capacity, cuco::empty_key(-1), MyKeyEqual(123));

I could write:

auto set = cuco::experimental::make_static_set(capacity, cuco::empty_key(-1), MyKeyEqual(123));

Notice how it's much smaller and I'm not repeating myself. Notice the addition of the work "make". This is consistent with the stdlib's usage of the word make in, for example, std::make_tuple

Describe alternatives you've considered

I can do without but its wordy. Doing this doesn't add much work for cuco. Cuco can modify some of its tests to use this form of the instantiation so that it is tested well in CI.

Additional context

Inference only works on functions, not classes: https://stackoverflow.com/questions/797594/when-a-compiler-can-infer-a-template-parameter

Here's how make_tuple works: https://en.cppreference.com/w/cpp/utility/tuple/make_tuple

@esoha-nvidia esoha-nvidia changed the title [ENHANCEMENT]: Helper functions for making a static_set et al [ENHANCEMENT]: Provide helper function cuco::experimental::make_static_set et al Sep 21, 2023
@jrhemstad
Copy link
Collaborator

Inference only works on functions, not classes: https://stackoverflow.com/questions/797594/when-a-compiler-can-infer-a-template-parameter

This is no longer true as of C++17 with "Class Template Argument Deduction" (CTAD). CTAD has all but made make_* functions obsolete.

@sleeepyjack
Copy link
Collaborator

Yes, the error you are seeing is due to us struggling with getting CTAD right 🙃
I wonder if this is something we can fix with a deduction guide?

@PointKernel
Copy link
Member

Yes, the error you are seeing is due to us struggling with getting CTAD right 🙃 I wonder if this is something we can fix with a deduction guide?

There are a few things in my mind to improve the situation. Wait for my PR(s)

@esoha-nvidia
Copy link
Author

I can test it very easily if you need. Let me know.

@PointKernel PointKernel added P2: Nice to have Desired, but not necessary type: improvement Improvement / enhancement to an existing function labels Sep 22, 2023
@PointKernel
Copy link
Member

PointKernel commented Sep 22, 2023

https://godbolt.org/z/4Y4WYfxcK

@esoha-nvidia actually, your example would deduce just fine even with the current implementation. Any other examples you would like to make them work?

@PointKernel
Copy link
Member

Updates: now with #346 being merged. Plain integers can be deduced as well.

https://godbolt.org/z/Wsbr17fh5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2: Nice to have Desired, but not necessary type: improvement Improvement / enhancement to an existing function
Projects
None yet
Development

No branches or pull requests

4 participants