Skip to content

<mdspan>: Improve debug checks - #3821

Merged
Stephan T. Lavavej (StephanTLavavej) merged 29 commits into
microsoft:feature/mdspan2from
JMazurkiewicz:mdspan/debug-guards
Jul 3, 2023
Merged

<mdspan>: Improve debug checks#3821
Stephan T. Lavavej (StephanTLavavej) merged 29 commits into
microsoft:feature/mdspan2from
JMazurkiewicz:mdspan/debug-guards

Conversation

@JMazurkiewicz

@JMazurkiewicz Jakub Mazurkiewicz (JMazurkiewicz) commented Jun 22, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Guard all existing debug checks with #if _CONTAINER_DEBUG_LEVEL > 0 (addresses <mdspan>: Completely rework std::extents #3586 (comment)),
  • extents:
    • Make _Static_extents array public and use it when possible,
    • Add new _Multidim_index_space_size_is_always_zero static member variable,
    • Split _Is_index_space_size_representable into two functions: _Is_static_multidim_index_space_size_representable and _Is_dynamic_multidim_index_space_size_representable,
    • Report DevCom-10398426 and silence it,
    • Improve debug check in extents(_OtherIndexTypes... _Exts) constructor,
  • layout_(left|right|stride)
    • Implement missing debug checks,
    • Replace scary looking immediately invoked lambda expressions with simple for loops (layout_(left|right)'s constructors from layout_stride),
    • Move precondition checks from operator() to corresponding _Index_impl functions (to avoid casting _Indices to index_type twice),
    • Drive-by: Remove unnecessary [[maybe_unused]] attributes in tests,
    • Drive-by: Simplify calls to check_members_with_various_extents in tests,
    • Drive-by: use ranges::adjacent_find instead of views::pairwise_transform in get_mapping_properties function,
    • Drive-by: fix exhaustiveness check in get_mapping_properties - previously, non-unique exhaustive mappings were recognized as non-exhaustive,
  • mdspan:
    • Implement missing debug checks,
    • Manually inline call to extents_type::static_extent in mdspan::static_extent and add precondition check,
    • Drive-by: improve performance of mdspan::empty,
    • Implement new death tests.

@JMazurkiewicz
Improve mdspan::empty
ff64bc8
@JMazurkiewicz
Improve _Is_index_space_size_representable and rename it to `_Is_st…
8d7468c 
…atic_multidim_index_space_size_representable`
@JMazurkiewicz
layout_left: Guard checks with #if _CONTAINER_DEBUG_LEVEL > 0
3e791db
@JMazurkiewicz
layout_left: Implement mapping(const extents_type&) constructor's…
9363526 
… precondition
@JMazurkiewicz
layout_right: Guard checks with #if _CONTAINER_DEBUG_LEVEL > 0
7c29805
@JMazurkiewicz
layout_right: Implement mapping(const extents_type&) constructor'…
e416aef 
…s precondition
@JMazurkiewicz
layout_stride: Guard checks with #if _CONTAINER_DEBUG_LEVEL > 0
6cd745b
@JMazurkiewicz
layout_left: Use for loop instead of huge lambda expression in pr…
e59d020 
…econdition check
@JMazurkiewicz
layout_right: Use for loop instead of huge lambda expression in p…
e1b3512 
…recondition check
@JMazurkiewicz
layout_stride: implement default constructor's preconditions
63e652b
@JMazurkiewicz
layout_stride: implement mapping(extents_type, array/span) constr…
1e5341e 
…uctor's preconditions
@JMazurkiewicz
extents: Improve extents(exts...) constructor's preconditions
6ce8adc
@JMazurkiewicz
mdspan: Implement mdspan(other mdspan) constructor's preconditions
f6aed8d
@JMazurkiewicz
extents: expose _Static_extents array
b3e4c4e
@JMazurkiewicz
mdspan: Implement precondition check in operator[]
360d1f7
@JMazurkiewicz
mdspan: Implement mdspan::size's preconditions
76e0498
@JMazurkiewicz
extents: Document DevCom-10398426 workaround
1a04c33
@JMazurkiewicz
layouts: fix _Contains_multidimensional_index checks
6044c69
@JMazurkiewicz
layout_stride: fix death test
77c4a0b
@JMazurkiewicz

This comment was marked as resolved.

Sign in to view
@JMazurkiewicz
Silence Clang's constexpr limit error in layout_(left|right)'s tests
eb1ee89 
Drive-by:
* Remove unnecessary `[[maybe_unused]]` attributes
* Simplify calls to `check_members_with_various_extents`
@JMazurkiewicz

This comment was marked as resolved.

Sign in to view
StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) approved these changes Jul 3, 2023
View reviewed changes
Comment thread stl/inc/mdspan
Comment thread tests/std/tests/P0009R18_mdspan_extents_death/test.cpp
Comment thread tests/std/tests/P0009R18_mdspan_layout_stride_death/test.cpp
Comment thread tests/std/tests/P0009R18_mdspan_mdspan_death/test.cpp
constexpr array<int, 256> some_ints{};

void test_construction_from_other_mdspan() {
auto mds1 = mdspan{some_ints.data(), 8, 2, 8};

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) Jul 3, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is an occurrence of "almost always auto" which we conventionally avoid. mdspan mds1{some_ints.data(), 8, 2, 8}; is the preferred form.

Occurs below.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 1984147 into microsoft:feature/mdspan2 Jul 3, 2023
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Jul 3, 2023

Copy link
Copy Markdown
Member

Thank you! I have minor comments for followup but nothing that prevented a merge.

@JMazurkiewicz
Jakub Mazurkiewicz (JMazurkiewicz) deleted the mdspan/debug-guards branch July 3, 2023 22:15
Jakub Mazurkiewicz (JMazurkiewicz) added a commit to JMazurkiewicz/STL that referenced this pull request Jul 5, 2023
@JMazurkiewicz
Grammar: as value -> as a value
d101073 
Addresses microsoft#3821 (comment)
Jakub Mazurkiewicz (JMazurkiewicz) added a commit to JMazurkiewicz/STL that referenced this pull request Jul 5, 2023
@JMazurkiewicz
Use "This test assumes that it isn't being compiled with /J"
a3d4af5 
Addresses microsoft#3821 (comment)
Jakub Mazurkiewicz (JMazurkiewicz) added a commit to JMazurkiewicz/STL that referenced this pull request Jul 5, 2023
@JMazurkiewicz
layout_right -> layout_stride
f2d0354 
Addresses microsoft#3821 (comment)
Jakub Mazurkiewicz (JMazurkiewicz) added a commit to JMazurkiewicz/STL that referenced this pull request Jul 5, 2023
@JMazurkiewicz
auto -> mdspan
805423a 
Addresses microsoft#3821 (comment)
Jakub Mazurkiewicz (JMazurkiewicz) added a commit to JMazurkiewicz/STL that referenced this pull request Jul 7, 2023
@JMazurkiewicz
Grammar: as value -> as a value
d3c4e46 
Addresses microsoft#3821 (comment)
Jakub Mazurkiewicz (JMazurkiewicz) added a commit to JMazurkiewicz/STL that referenced this pull request Jul 7, 2023
@JMazurkiewicz
Use "This test assumes that it isn't being compiled with /J"
27e8937 
Addresses microsoft#3821 (comment)
Jakub Mazurkiewicz (JMazurkiewicz) added a commit to JMazurkiewicz/STL that referenced this pull request Jul 7, 2023
@JMazurkiewicz
layout_right -> layout_stride
82116f9 
Addresses microsoft#3821 (comment)
Jakub Mazurkiewicz (JMazurkiewicz) added a commit to JMazurkiewicz/STL that referenced this pull request Jul 7, 2023
@JMazurkiewicz
auto -> mdspan
7fcd00a 
Addresses microsoft#3821 (comment)
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) StephanTLavavej approved these changes

Assignees

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej)

Labels

mdspan C++23 mdspan

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JMazurkiewicz @StephanTLavavej