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

<xutility>, <algorithm>: vector algorithms dispatching functions miss top level const on pointer parameters #4385

Closed
AlexGuteniev opened this issue Feb 10, 2024 · 3 comments · Fixed by #4410
Assignees
Labels
enhancement Something can be improved fixed Something works now, yay! good first issue Good for newcomers

Comments

@AlexGuteniev
Copy link
Contributor

AlexGuteniev commented Feb 10, 2024

The affected code

STL/stl/inc/algorithm

Lines 66 to 112 in bd3d740

_STD_BEGIN
template <class _Ty>
_STD pair<_Ty*, _Ty*> __std_minmax_element(_Ty* _First, _Ty* _Last) noexcept {
constexpr bool _Signed = _STD is_signed_v<_Ty>;
_Min_max_element_t _Res;
if constexpr (_STD is_same_v<_STD remove_const_t<_Ty>, float>) {
_Res = ::__std_minmax_element_f(_First, _Last, false);
} else if constexpr (_STD _Is_any_of_v<_STD remove_const_t<_Ty>, double, long double>) {
_Res = ::__std_minmax_element_d(_First, _Last, false);
} else if constexpr (sizeof(_Ty) == 1) {
_Res = ::__std_minmax_element_1(_First, _Last, _Signed);
} else if constexpr (sizeof(_Ty) == 2) {
_Res = ::__std_minmax_element_2(_First, _Last, _Signed);
} else if constexpr (sizeof(_Ty) == 4) {
_Res = ::__std_minmax_element_4(_First, _Last, _Signed);
} else if constexpr (sizeof(_Ty) == 8) {
_Res = ::__std_minmax_element_8(_First, _Last, _Signed);
} else {
static_assert(_STD _Always_false<_Ty>, "Unexpected size");
}
return {const_cast<_Ty*>(static_cast<const _Ty*>(_Res._Min)), const_cast<_Ty*>(static_cast<const _Ty*>(_Res._Max))};
}
template <class _Ty, class _TVal>
_Ty* __std_find_last_trivial(_Ty* _First, _Ty* _Last, const _TVal _Val) noexcept {
if constexpr (_STD is_pointer_v<_TVal> || _STD is_null_pointer_v<_TVal>) {
return _STD __std_find_last_trivial(_First, _Last, reinterpret_cast<uintptr_t>(_Val));
} else if constexpr (sizeof(_Ty) == 1) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_last_trivial_1(_First, _Last, static_cast<uint8_t>(_Val))));
} else if constexpr (sizeof(_Ty) == 2) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_last_trivial_2(_First, _Last, static_cast<uint16_t>(_Val))));
} else if constexpr (sizeof(_Ty) == 4) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_last_trivial_4(_First, _Last, static_cast<uint32_t>(_Val))));
} else if constexpr (sizeof(_Ty) == 8) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_last_trivial_8(_First, _Last, static_cast<uint64_t>(_Val))));
} else {
static_assert(_STD _Always_false<_Ty>, "Unexpected size");
}
}
_STD_END

STL/stl/inc/xutility

Lines 113 to 214 in bd3d740

_STD_BEGIN
template <class _Ty, class _TVal>
__declspec(noalias) size_t __std_count_trivial(_Ty* _First, _Ty* _Last, const _TVal _Val) noexcept {
if constexpr (_STD is_pointer_v<_TVal> || _STD is_null_pointer_v<_TVal>) {
return _STD __std_count_trivial(_First, _Last, reinterpret_cast<uintptr_t>(_Val));
} else if constexpr (sizeof(_Ty) == 1) {
return ::__std_count_trivial_1(_First, _Last, static_cast<uint8_t>(_Val));
} else if constexpr (sizeof(_Ty) == 2) {
return ::__std_count_trivial_2(_First, _Last, static_cast<uint16_t>(_Val));
} else if constexpr (sizeof(_Ty) == 4) {
return ::__std_count_trivial_4(_First, _Last, static_cast<uint32_t>(_Val));
} else if constexpr (sizeof(_Ty) == 8) {
return ::__std_count_trivial_8(_First, _Last, static_cast<uint64_t>(_Val));
} else {
static_assert(_STD _Always_false<_Ty>, "Unexpected size");
}
}
template <class _Ty, class _TVal>
_Ty* __std_find_trivial(_Ty* _First, _Ty* _Last, const _TVal _Val) noexcept {
if constexpr (_STD is_pointer_v<_TVal> || _STD is_null_pointer_v<_TVal>) {
return _STD __std_find_trivial(_First, _Last, reinterpret_cast<uintptr_t>(_Val));
} else if constexpr (sizeof(_Ty) == 1) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_trivial_1(_First, _Last, static_cast<uint8_t>(_Val))));
} else if constexpr (sizeof(_Ty) == 2) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_trivial_2(_First, _Last, static_cast<uint16_t>(_Val))));
} else if constexpr (sizeof(_Ty) == 4) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_trivial_4(_First, _Last, static_cast<uint32_t>(_Val))));
} else if constexpr (sizeof(_Ty) == 8) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_trivial_8(_First, _Last, static_cast<uint64_t>(_Val))));
} else {
static_assert(_STD _Always_false<_Ty>, "Unexpected size");
}
}
template <class _Ty, class _TVal>
_Ty* __std_find_trivial_unsized(_Ty* _First, const _TVal _Val) noexcept {
if constexpr (_STD is_pointer_v<_TVal> || _STD is_null_pointer_v<_TVal>) {
return _STD __std_find_trivial_unsized(_First, reinterpret_cast<uintptr_t>(_Val));
} else if constexpr (sizeof(_Ty) == 1) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_trivial_unsized_1(_First, static_cast<uint8_t>(_Val))));
} else if constexpr (sizeof(_Ty) == 2) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_trivial_unsized_2(_First, static_cast<uint16_t>(_Val))));
} else if constexpr (sizeof(_Ty) == 4) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_trivial_unsized_4(_First, static_cast<uint32_t>(_Val))));
} else if constexpr (sizeof(_Ty) == 8) {
return const_cast<_Ty*>(
static_cast<const _Ty*>(::__std_find_trivial_unsized_8(_First, static_cast<uint64_t>(_Val))));
} else {
static_assert(_STD _Always_false<_Ty>, "Unexpected size");
}
}
template <class _Ty>
_Ty* __std_min_element(_Ty* _First, _Ty* _Last) noexcept {
constexpr bool _Signed = _STD is_signed_v<_Ty>;
if constexpr (_STD is_same_v<_STD remove_const_t<_Ty>, float>) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_min_element_f(_First, _Last, false)));
} else if constexpr (_STD _Is_any_of_v<_STD remove_const_t<_Ty>, double, long double>) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_min_element_d(_First, _Last, false)));
} else if constexpr (sizeof(_Ty) == 1) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_min_element_1(_First, _Last, _Signed)));
} else if constexpr (sizeof(_Ty) == 2) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_min_element_2(_First, _Last, _Signed)));
} else if constexpr (sizeof(_Ty) == 4) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_min_element_4(_First, _Last, _Signed)));
} else if constexpr (sizeof(_Ty) == 8) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_min_element_8(_First, _Last, _Signed)));
} else {
static_assert(_STD _Always_false<_Ty>, "Unexpected size");
}
}
template <class _Ty>
_Ty* __std_max_element(_Ty* _First, _Ty* _Last) noexcept {
constexpr bool _Signed = _STD is_signed_v<_Ty>;
if constexpr (_STD is_same_v<_STD remove_const_t<_Ty>, float>) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_max_element_f(_First, _Last, false)));
} else if constexpr (_STD _Is_any_of_v<_STD remove_const_t<_Ty>, double, long double>) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_max_element_d(_First, _Last, false)));
} else if constexpr (sizeof(_Ty) == 1) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_max_element_1(_First, _Last, _Signed)));
} else if constexpr (sizeof(_Ty) == 2) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_max_element_2(_First, _Last, _Signed)));
} else if constexpr (sizeof(_Ty) == 4) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_max_element_4(_First, _Last, _Signed)));
} else if constexpr (sizeof(_Ty) == 8) {
return const_cast<_Ty*>(static_cast<const _Ty*>(::__std_max_element_8(_First, _Last, _Signed)));
} else {
static_assert(_STD _Always_false<_Ty>, "Unexpected size");
}
}
_STD_END

The desired change

The functions being defined here are taking _Ty* _First, _Ty* _Last parameters, but aren't modifying _First and _Last themselves. We should apply top-level const to these pointers, changing them to _Ty* const _First, _Ty* const _Last which will make our code more consistent and ever-so-slightly more resistant to mistakes.

Note that __std_find_trivial_unsized takes only _Ty* _First but should be changed similarly.

About this "good first issue"

This issue is intended for a new contributor (especially one new to GitHub) to get started with a simple change.

Please feel free to submit a pull request if there isn't one already linked here - no need to ask for permission! 😸

You can (and should) link your pull request to this issue using GitHub's close/fix/resolve syntax.
(in the PR description not the commit message)

@CaseyCarter CaseyCarter added enhancement Something can be improved add context With more explanation, could be good first issue labels Feb 12, 2024
@StephanTLavavej StephanTLavavej added good first issue Good for newcomers and removed add context With more explanation, could be good first issue labels Feb 14, 2024
@KamathScience
Copy link
Contributor

Hi, I am new to open-source development and interested in working on this. Could you please assign this to me?

@StephanTLavavej
Copy link
Member

Done! We don't usually assign issues to contributors - it's usually sufficient for a contributor to comment "I'm looking into this" to avoid duplicating work while they prepare a PR - but for things labeled "good first issue" I can see how having an actual assignment will help people searching for unassigned issues.

Welcome to the open-source world! 🐱 🗺️

@AlexGuteniev
Copy link
Contributor Author

Due to the latest merge, the scope is now few functions more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved fixed Something works now, yay! good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants