Skip to content

Commit

Permalink
Fix and improve array and mdspan static analysis warnings (#4856)
Browse files Browse the repository at this point in the history
  • Loading branch information
StephanTLavavej committed Aug 8, 2024
1 parent 3edd71e commit 0f8f6be
Show file tree
Hide file tree
Showing 17 changed files with 155 additions and 61 deletions.
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 @@ -992,22 +992,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 @@ -1086,6 +1079,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 @@ -758,4 +758,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
3 changes: 3 additions & 0 deletions tests/std/tests/P0009R18_mdspan_layout_stride_death/test.cpp
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

0 comments on commit 0f8f6be

Please sign in to comment.