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

Move comparison function objects out of priority queue header #466

Merged
merged 3 commits into from
Jan 21, 2021

Conversation

dalg24
Copy link
Contributor

@dalg24 dalg24 commented Jan 21, 2021

Rational: It somehow bothered me when we had to include <ArborX_DetailsPriorityQueue.hpp> to use Less in #453

Note that I made the operators constexpr to be consistent with the function objects in <functional>

@dalg24 dalg24 added the refactoring Code reorganization label Jan 21, 2021
Copy link
Contributor

@aprokop aprokop left a comment

Choose a reason for hiding this comment

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

I agree with the idea. I'm unused to "function object", and not sure what it is. If it is not commonly used, I would prefer something else like ArborX_DetailsComparisonFunctors.hpp.

And do you envision the scope of this file to expand in the future to include something else?

@dalg24
Copy link
Contributor Author

dalg24 commented Jan 21, 2021

I agree with the idea. I'm unused to "function object", and not sure what it is. If it is not commonly used, I would prefer something else like ArborX_DetailsComparisonFunctors.hpp.

https://eel.is/c++draft/function.objects

And do you envision the scope of this file to expand in the future to include something else?

Scope no. More function objects maybe.

I should have commented that if it wasn't for NVCC we would use them directly from the standard.
https://docs.nvidia.com/cuda/cuda-compiler-driver-nvcc/index.html#options-for-altering-compiler-linker-behavior-expt-relaxed-constexpr

@dalg24
Copy link
Contributor Author

dalg24 commented Jan 21, 2021

I agree with the idea. I'm unused to "function object", and not sure what it is. If it is not commonly used, I would prefer something else like ArborX_DetailsComparisonFunctors.hpp.

https://eel.is/c++draft/function.objects

And do you envision the scope of this file to expand in the future to include something else?

Scope no. More function objects maybe.

If you feel strongly about it I can change. The name your suggest works. I went for something more generic because I felt if we were to need std::plus I would not want to have it in a separate header.

@aprokop aprokop merged commit f25ca28 into arborx:master Jan 21, 2021
@dalg24 dalg24 deleted the function_objects branch January 21, 2021 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code reorganization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants