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

Fix and improve array and mdspan static analysis warnings #4856

Merged
merged 7 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions stl/inc/array
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ public:
return _Elems + _Size;
}

_NODISCARD constexpr size_type size() const noexcept {
_NODISCARD _Ret_range_(==, _Size) constexpr size_type size() const noexcept {
return _Size;
}

Expand Down Expand Up @@ -528,15 +528,15 @@ public:
return _Elems[_Pos];
}

_NODISCARD _CONSTEXPR17 reference operator[](_In_range_(0, _Size - 1) size_type _Pos) noexcept /* strengthened */ {
_NODISCARD _CONSTEXPR17 reference operator[](_In_range_(<, _Size) size_type _Pos) noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Pos < _Size, "array subscript out of range");
#endif // _CONTAINER_DEBUG_LEVEL > 0

return _Elems[_Pos];
}

_NODISCARD constexpr const_reference operator[](_In_range_(0, _Size - 1) size_type _Pos) const noexcept
_NODISCARD constexpr const_reference operator[](_In_range_(<, _Size) size_type _Pos) const noexcept
/* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Pos < _Size, "array subscript out of range");
Expand Down
24 changes: 14 additions & 10 deletions stl/inc/mdspan
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ private:
rank_type _Counter = 0;
for (rank_type _Idx = 0; _Idx < _Rank; ++_Idx) {
if (_Static_extents[_Idx] == dynamic_extent) {
_Analysis_assume_(_Counter < _Rank_dynamic); // TRANSITION, DevCom-923103
_Analysis_assume_(_Counter < _Rank_dynamic); // guaranteed by how _Rank_dynamic is calculated
_Result[_Counter] = _Idx;
++_Counter;
}
Expand Down Expand Up @@ -174,22 +174,22 @@ private:
}

public:
_NODISCARD static constexpr rank_type rank() noexcept {
_NODISCARD _Ret_range_(==, _Rank) static constexpr rank_type rank() noexcept {
return _Rank;
}

_NODISCARD static constexpr rank_type rank_dynamic() noexcept {
return _Rank_dynamic;
}

_NODISCARD static constexpr size_t static_extent(const rank_type _Idx) noexcept {
_NODISCARD static constexpr size_t static_extent(_In_range_(<, _Rank) const rank_type _Idx) noexcept {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Idx < _Rank, "Index must be less than rank() (N4950 [mdspan.extents.obs]/1)");
#endif // _CONTAINER_DEBUG_LEVEL > 0
return _Static_extents[_Idx];
}

_NODISCARD constexpr index_type extent(const rank_type _Idx) const noexcept {
_NODISCARD constexpr index_type extent(_In_range_(<, _Rank) const rank_type _Idx) const noexcept {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Idx < _Rank, "Index must be less than rank() (N4950 [mdspan.extents.obs]/3)");
#endif // _CONTAINER_DEBUG_LEVEL > 0
Expand Down Expand Up @@ -591,7 +591,7 @@ public:
return true;
}

_NODISCARD constexpr index_type stride(const rank_type _Idx) const noexcept
_NODISCARD constexpr index_type stride(_In_range_(<, extents_type::_Rank) const rank_type _Idx) const noexcept
requires (extents_type::rank() > 0)
{
#if _CONTAINER_DEBUG_LEVEL > 0
Expand Down Expand Up @@ -744,7 +744,7 @@ public:
return true;
}

_NODISCARD constexpr index_type stride(const rank_type _Idx) const noexcept
_NODISCARD constexpr index_type stride(_In_range_(<, extents_type::_Rank) const rank_type _Idx) const noexcept
requires (extents_type::rank() > 0)
{
#if _CONTAINER_DEBUG_LEVEL > 0
Expand Down Expand Up @@ -975,10 +975,14 @@ public:
return true;
}

_NODISCARD constexpr index_type stride(const rank_type _Idx) const noexcept {
_NODISCARD constexpr index_type stride(_In_range_(<, extents_type::_Rank) const rank_type _Idx) const noexcept {
if constexpr (extents_type::rank() == 0) {
_STL_VERIFY(false, "The argument to stride must be nonnegative and less than extents_type::rank().");
} else {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Idx < extents_type::_Rank,
"The argument to stride must be nonnegative and less than extents_type::rank().");
#endif // _CONTAINER_DEBUG_LEVEL > 0
return this->_Array[_Idx];
}
}
Expand Down Expand Up @@ -1187,22 +1191,22 @@ private:
"[mdspan.mdspan.overview]/2.3).");

public:
_NODISCARD static constexpr rank_type rank() noexcept {
_NODISCARD _Ret_range_(==, extents_type::_Rank) static constexpr rank_type rank() noexcept {
return extents_type::_Rank;
}

_NODISCARD static constexpr rank_type rank_dynamic() noexcept {
return extents_type::_Rank_dynamic;
}

_NODISCARD static constexpr size_t static_extent(const rank_type _Idx) noexcept {
_NODISCARD static constexpr size_t static_extent(_In_range_(<, extents_type::_Rank) const rank_type _Idx) noexcept {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Idx < extents_type::_Rank, "Index must be less than rank() (N4950 [mdspan.extents.obs]/1)");
#endif // _CONTAINER_DEBUG_LEVEL > 0
return extents_type::_Static_extents[_Idx];
}

_NODISCARD constexpr index_type extent(const rank_type _Idx) const noexcept {
_NODISCARD constexpr index_type extent(_In_range_(<, extents_type::_Rank) const rank_type _Idx) const noexcept {
return this->_Map.extents().extent(_Idx);
}

Expand Down
15 changes: 5 additions & 10 deletions tests/libcxx/expected_results.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1006,22 +1006,15 @@ std/utilities/format/format.range/format.range.fmtset/format.pass.cpp FAIL
std/utilities/format/format.range/format.range.fmtstr/format.pass.cpp FAIL
std/utilities/format/format.tuple/format.pass.cpp FAIL

# Not analyzed. Apparent false positives from static analysis where it thinks that array indexing is out of bounds.
# warning C28020: The expression '0<=_Param_(1)&&_Param_(1)<=1-1' is not true at this call.
# Not analyzed. Static analysis thinks that array indexing is out of bounds because it can't prove otherwise.
# warning C28020: The expression '_Param_(1)<1' is not true at this call.
# Note: The :1 (ASAN) configuration doesn't run static analysis.
std/containers/views/mdspan/extents/ctor_default.pass.cpp:0 FAIL
std/containers/views/mdspan/extents/ctor_from_array.pass.cpp:0 FAIL
std/containers/views/mdspan/extents/ctor_from_integral.pass.cpp:0 FAIL
std/containers/views/mdspan/extents/ctor_from_span.pass.cpp:0 FAIL
std/containers/views/mdspan/layout_left/ctor.layout_stride.pass.cpp:0 FAIL
std/containers/views/mdspan/layout_left/stride.pass.cpp:0 FAIL
std/containers/views/mdspan/layout_right/ctor.layout_stride.pass.cpp:0 FAIL
std/containers/views/mdspan/layout_right/stride.pass.cpp:0 FAIL
std/containers/views/mdspan/layout_stride/ctor.default.pass.cpp:0 FAIL
std/containers/views/mdspan/layout_stride/ctor.extents_array.pass.cpp:0 FAIL
std/containers/views/mdspan/layout_stride/ctor.extents_span.pass.cpp:0 FAIL
std/containers/views/mdspan/layout_stride/comparison.pass.cpp:0 FAIL
std/containers/views/mdspan/layout_stride/ctor.strided_mapping.pass.cpp:0 FAIL
std/containers/views/mdspan/layout_stride/stride.pass.cpp:0 FAIL
std/containers/views/mdspan/mdspan/conversion.pass.cpp:0 FAIL
std/containers/views/mdspan/mdspan/ctor.dh_array.pass.cpp:0 FAIL
std/containers/views/mdspan/mdspan/ctor.dh_span.pass.cpp:0 FAIL
Expand Down Expand Up @@ -1099,6 +1092,8 @@ std/algorithms/alg.modifying.operations/alg.transform/ranges.transform.binary.ra
std/algorithms/alg.modifying.operations/alg.transform/ranges.transform.binary.range.pass.cpp:1 SKIPPED
std/algorithms/alg.nonmodifying/alg.ends_with/ranges.ends_with.pass.cpp:0 SKIPPED
std/algorithms/alg.nonmodifying/alg.ends_with/ranges.ends_with.pass.cpp:1 SKIPPED
std/algorithms/alg.sorting/alg.lex.comparison/ranges.lexicographical_compare.pass.cpp:0 SKIPPED
std/algorithms/alg.sorting/alg.lex.comparison/ranges.lexicographical_compare.pass.cpp:1 SKIPPED
std/algorithms/alg.sorting/alg.merge/ranges_merge.pass.cpp:0 SKIPPED
std/algorithms/alg.sorting/alg.merge/ranges_merge.pass.cpp:1 SKIPPED
std/algorithms/alg.sorting/alg.set.operations/includes/ranges_includes.pass.cpp:0 SKIPPED
Expand Down
5 changes: 4 additions & 1 deletion tests/std/include/test_mdspan_support.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,10 @@ MappingProperties<Mapping> get_mapping_properties(const Mapping& mapping) {
constexpr auto rank = Mapping::extents_type::rank();
constexpr std::make_index_sequence<rank> rank_indices;

auto get_extent = [&](size_t i) { return mapping.extents().extent(i); };
auto get_extent = [&](size_t i) {
assert(i < rank);
return mapping.extents().extent(i);
};
auto multidim_indices = [&]<size_t... Indices>(std::index_sequence<Indices...>) {
return std::views::cartesian_product(std::views::iota(IndexType{0}, get_extent(Indices))...);
}(rank_indices);
Expand Down
1 change: 1 addition & 0 deletions tests/std/test.lst
Original file line number Diff line number Diff line change
Expand Up @@ -755,4 +755,5 @@ tests\VSO_0849827_multicontainer_emplace_hint_position
tests\VSO_0938757_attribute_order
tests\VSO_0961751_hash_range_erase
tests\VSO_0971246_legacy_await_headers
tests\VSO_1804139_static_analysis_warning_with_single_element_array
tests\VSO_1925201_iter_traits
6 changes: 6 additions & 0 deletions tests/std/tests/P0009R18_mdspan_extents_death/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,19 @@ using namespace std;
void test_static_extent_function_with_invalid_index() {
using E = extents<int, 3>;
// Index must be less than rank()
#pragma warning(push)
#pragma warning(disable : 28020) // yay, /analyze catches this mistake at compile time!
(void) E::static_extent(1);
#pragma warning(pop)
}

void test_extent_function_with_invalid_index() {
extents<int, 3> e;
// Index must be less than rank()
#pragma warning(push)
#pragma warning(disable : 28020) // yay, /analyze catches this mistake at compile time!
(void) e.extent(1);
#pragma warning(pop)
}

void test_construction_from_other_extents_with_invalid_values() {
Expand Down
3 changes: 0 additions & 3 deletions tests/std/tests/P0009R18_mdspan_layout_left/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,7 @@ constexpr void check_members(const extents<IndexType, Extents...>& ext, index_se
if constexpr (Ext::rank() > 0) {
strides.front() = 1;
for (size_t i = 1; i < Ext::rank(); ++i) {
#pragma warning(push)
#pragma warning(disable : 28020) // TRANSITION, DevCom-923103
strides[i] = static_cast<IndexType>(strides[i - 1] * ext.extent(i - 1));
#pragma warning(pop)
}
}

Expand Down
3 changes: 3 additions & 0 deletions tests/std/tests/P0009R18_mdspan_layout_left_death/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ void test_call_operator() {
void test_stride_function() {
layout_left::mapping<extents<int, 3>> m;
// Value of i must be less than extents_type::rank()
#pragma warning(push)
#pragma warning(disable : 28020) // yay, /analyze catches this mistake at compile time!
(void) m.stride(1);
#pragma warning(pop)
}

int main(int argc, char* argv[]) {
Expand Down
3 changes: 0 additions & 3 deletions tests/std/tests/P0009R18_mdspan_layout_right/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,7 @@ constexpr void check_members(const extents<IndexType, Extents...>& ext, index_se
if constexpr (Ext::rank() > 0) {
strides.back() = 1;
for (size_t i = Ext::rank() - 1; i-- > 0;) {
#pragma warning(push)
#pragma warning(disable : 28020) // TRANSITION, DevCom-923103
strides[i] = static_cast<IndexType>(strides[i + 1] * ext.extent(i + 1));
#pragma warning(pop)
}
}

Expand Down
3 changes: 3 additions & 0 deletions tests/std/tests/P0009R18_mdspan_layout_right_death/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ void test_call_operator() {
void test_stride_function() {
layout_right::mapping<extents<int, 3>> m;
// Value of i must be less than extents_type::rank()
#pragma warning(push)
#pragma warning(disable : 28020) // yay, /analyze catches this mistake at compile time!
(void) m.stride(1);
#pragma warning(pop)
}

int main(int argc, char* argv[]) {
Expand Down
5 changes: 1 addition & 4 deletions tests/std/tests/P0009R18_mdspan_layout_stride/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,9 @@ constexpr void do_check_members(const extents<IndexType, Extents...>& ext,
}

{ // Check 'stride' function
for (size_t i = 0; i < strs.size(); ++i) {
for (size_t i = 0; i < Ext::rank(); ++i) {
same_as<IndexType> decltype(auto) s = m.stride(i);
#pragma warning(push)
#pragma warning(disable : 28020) // TRANSITION, DevCom-923103
assert(cmp_equal(strs[i], s));
#pragma warning(pop)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ void test_call_operator() {
void test_stride_with_empty_extents() {
layout_stride::mapping<extents<int>> m;
// The argument to stride must be nonnegative and less than extents_type::rank()
#pragma warning(push)
#pragma warning(disable : 28020) // yay, /analyze catches this mistake at compile time!
(void) m.stride(0);
#pragma warning(pop)
}

int main(int argc, char* argv[]) {
Expand Down
7 changes: 4 additions & 3 deletions tests/std/tests/P0218R1_filesystem/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3859,11 +3859,12 @@ basic_ostream<Elem, Traits>& operator<<(basic_ostream<Elem, Traits>& ostr, const
L"symlink"sv, L"block"sv, L"character"sv, L"fifo"sv, L"socket"sv, L"unknown"sv, L"junction"sv}};

const size_t index = static_cast<size_t>(ft);
if (!EXPECT(index < names.size())) {
if (index < names.size()) {
return ostr << L"file_type::" << names[index];
} else {
EXPECT(false);
return ostr << L"!!! INVALID file_type(" << index << L") !!!!";
}

return ostr << L"file_type::" << names[index];
}

template <typename Elem, typename Traits>
Expand Down
Loading