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

Add missing min/max, comparison and permutation operations of C++ algorithm library #4448

Merged
merged 7 commits into from
Nov 11, 2021

Conversation

jhelgert
Copy link
Contributor

@jhelgert jhelgert commented Nov 4, 2021

Adds most of the missing minimum/maximum operations, comparison operations and permutation operations of the algorithm library (<= C++17):

  • minmax, minmax_element (C++11), clamp (C++17) and one missing overload of min/max_element.
  • equal, lexicographical_compare
  • is_permutation, next_permutation, prev_permutation

@jhelgert jhelgert changed the title Add missing min/max methods of C++ algorithm library Add missing min/max, comparison and permutation operations of C++ algorithm library Nov 9, 2021
@jhelgert
Copy link
Contributor Author

jhelgert commented Nov 9, 2021

Hmm, I can reproduce the failing tests on macOS with Apple Clang++ 12.0.0:

#include <vector>
#include <algorithm>

bool comp(int a, int b){ return a == b; }

int main(){
	std::vector<int> v1{1,2,3,4}, v2{4,2,3,1};

	// 2
	std::is_permutation<std::vector<int>::iterator, std::vector<int>::iterator, bool (int, int)>(v1.begin(), v1.end(), v2.begin(), comp);

	// 3
	std::is_permutation<std::vector<int>::iterator, std::vector<int>::iterator, bool (int, int)>(v1.begin(), v1.end(), v2.begin(), v2.end(), comp);
	
	return 0;
}

compiling with -std=c++14 -Wall flags yields

In file included from /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/vector:274:
In file included from /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/__bit_reference:15:
/Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/algorithm:1471:12: error: no matching function for call to '__is_permutation'
    return _VSTD::__is_permutation<typename add_lvalue_reference<_BinaryPredicate>::type>
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/__config:840:15: note: expanded from macro '_VSTD'
#define _VSTD std::_LIBCPP_ABI_NAMESPACE
              ^
/Users/jonathan/Desktop/bla.cpp:17:7: note: in instantiation of function template specialization 'std::__1::is_permutation<std::__1::__wrap_iter<int *>, std::__1::__wrap_iter<int *>, bool (int, int)>' requested here
        std::is_permutation<std::vector<int>::iterator, 
             ^
/Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/algorithm:1451:1: note: candidate function template not viable: no known conversion from 'bool (*)(int, int)' to 'bool (&)(int, int)' for 5th argument; dereference the argument with *
__is_permutation(_RandomAccessIterator1 __first1, _RandomAccessIterator2 __last1,
^
/Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/algorithm:1398:1: note: candidate function template not viable: no known conversion from 'bool (*)(int, int)' to 'bool (&)(int, int)' for 5th argument; dereference the argument with *
__is_permutation(_ForwardIterator1 __first1, _ForwardIterator1 __last1,
^
1 error generated.

However, It compiles without any warnings on macOS with gcc-11. The same holds for non-Apple Clang, see the godbolt snippet here.

EDIT: Just noticed that it compiles fine with Apple Clang using the bool (*)(int, int) type as the last template argument.

@scoder Any ideas on how we should handle this?

@da-woods
Copy link
Contributor

da-woods commented Nov 9, 2021

@jhelgert
Copy link
Contributor Author

What do you think? Is it ready to be merged?

@scoder scoder merged commit ee7f61a into cython:master Nov 11, 2021
@scoder
Copy link
Contributor

scoder commented Nov 11, 2021

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants