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

In subspan function, add static checks on extents #843

Conversation

mike919192
Copy link
Contributor

Potential fix for issue #842

etl::span subspan function does not currently statically check extents. std::span does check this.

Code demonstration:

std::array<int, 10> array {0};

//std::span std_span1 = std::span(array).subspan<15, 1>(); //compile time error
//std::span std_span2 = std::span(array).subspan<5, 8>(); //compile time error

etl::span etl_span1 = etl::span(array).subspan<15, 1>(); //currently no compile time error
etl::span etl_span2 = etl::span(array).subspan<5, 8>(); //currently no compile time error

This PR should enable c++11 and newer to have compile error when the extent check fails.

Copy link

Review changes with SemanticDiff.

@jwellbelove jwellbelove changed the base branch from master to pull-request/#843-In-subspan-function,-add-static-checks-on-extents February 15, 2024 11:11
Copy link
Contributor

@jwellbelove jwellbelove left a comment

Choose a reason for hiding this comment

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

static_assert should be replaced with ETL_STATIC_ASSERT.

Copy link
Contributor

@jwellbelove jwellbelove left a comment

Choose a reason for hiding this comment

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

The static asserts have no message fields

@mike919192
Copy link
Contributor Author

I also realized that the first and last functions also behave similarly, so I have also added the checks for those functions. I believe those are all the functions where static checks are possible.

std::array<int, 10> array {0};

//std::span std_span1 = std::span(array).first<15>(); //compile time error
//std::span std_span2 = std::span(array).last<15>(); //compile time error

etl::span etl_span1 = etl::span(array).first<15>(); //currently no compile time error
etl::span etl_span2 = etl::span(array).last<15>(); //currently no compile time error

I don't know if it makes sense to also add the static assert for the pre c++11 subspan function? I'll await @jwellbelove feedback.

@jwellbelove
Copy link
Contributor

The ETL_STATIC_ASSERT will work for pre-C++11 as the macro defines a C++03 static assert implementation.

@mike919192
Copy link
Contributor Author

Thanks. I duplicated the static asserts for the pre c++11 subspan function. I should be finished with changes unless there are other issues.

@jwellbelove jwellbelove merged commit 4b12e98 into ETLCPP:pull-request/#843-In-subspan-function,-add-static-checks-on-extents Feb 16, 2024
63 checks passed
jwellbelove pushed a commit that referenced this pull request Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants