-
-
Couldn't load subscription status.
- Fork 473
etl::span checks #1201
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
etl::span checks #1201
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Walkthrough
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/test_span_fixed_extent.cpp (1)
431-439: Tests updated for span_out_of_range and extra checks — looks goodRuntime OOB checks now expect etl::span_out_of_range and static-assert cases are left commented. This aligns with the new span behaviour and checking macros.
Consider pruning or moving the commented static-assert demos to a dedicated compile-time test block to keep the runtime suite concise.
Also applies to: 463-465, 479-481, 564-567, 589-591, 615-618, 640-642, 697-705, 706-711
test/test_span_dynamic_extent.cpp (1)
714-718: Undef helper macros after use to avoid leakage across the TUDefine/undef within the test to prevent unintended reuse later in the file.
Apply this small cleanup:
- #define SPAN6_EXPR { auto span6 = view.subspan<2, 7>(); (void)span6; } + #define SPAN6_EXPR { auto span6 = view.subspan<2, 7>(); (void)span6; } CHECK_THROW(SPAN6_EXPR, etl::span_out_of_range); - #define CSPAN6_EXPR { auto cspan6 = cview.subspan<2, 7>(); (void)cspan6; } + #define CSPAN6_EXPR { auto cspan6 = cview.subspan<2, 7>(); (void)cspan6; } CHECK_THROW(CSPAN6_EXPR, etl::span_out_of_range); + #undef SPAN6_EXPR + #undef CSPAN6_EXPR
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
include/etl/array.h(1 hunks)include/etl/error_handler.h(1 hunks)include/etl/span.h(18 hunks)test/test_span_dynamic_extent.cpp(8 hunks)test/test_span_fixed_extent.cpp(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
include/etl/*.h
⚙️ CodeRabbit configuration file
include/etl/*.h: Review the C++ code in these directories. The code must not use
STL containers but must instead rely on Embedded Template Library
(ETL) for data structures and algorithms.When answering questions, provide accurate and concise information based on the ETL documentation and codebase. If you don't know the answer, just say "I don't know". Do not make up answers.
When providing code examples, ensure they are relevant to the ETL and follow its conventions.
Always be polite and professional in your responses.Header files should start with the ETL's standard MIT comment block.
Header files should have include guards in the format '#ifndef ETL_>INCLUDED #define ETL_INCLUDED #endif'
Headers should be organized in the following order
- The first header include should be '#include "platform.h"'
- Headers from the ETL
- Headers from the path 'private/**'
- C++ standard library headers
- C standard library headers
Pay extra attention to the following
- Buffer overflows
- Memory leaks
- Undefined behavior
Check that the following conventions are used
- Variable names should be in
snake_case- Macro constants should be
ALL_CAPS_WITH_UNDERSCORES- Macro constants should be prefixed with 'ETL_'
- Function, class, and type names should be
snake_case- enum members should be
Caps_Snake_CasePlease keep your outputs short and to the point, unless otherwise asked
- Keep your responses short, describe small issues in a few sentences
- Don't output tips, analysis chains or anything else with a collapsible view
- Don't output
comment summariesorfix prompts for AIs- If the highlighted issue is complex, don't suggest a solution
- Don't use emojis
Files:
include/etl/array.hinclude/etl/error_handler.hinclude/etl/span.h
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Clang C++23 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++23 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++20 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC C++20 Linux - STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC C++20 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++20 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++20 Linux - STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Clang C++20 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++20 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++14 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++17 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++14 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++17 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++14 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++14 Linux - No STL (ubuntu-22.04)
🔇 Additional comments (6)
include/etl/array.h (1)
162-168: Operator[] C++11 throw path correctly gated by new checking-state macroCondition switch to ETL_NOT_USING_CPP14 and ETL_CHECKING_INDEX_OPERATOR is correct and consistent with new error handler macros.
include/etl/error_handler.h (1)
378-385: State macros for index/extra checks are well introducedETL_CHECKING_INDEX_OPERATOR / ETL_CHECKING_EXTRA (and complements) are correctly defined in both branches, enabling clear downstream conditionals.
Also applies to: 389-396
test/test_span_dynamic_extent.cpp (1)
442-449: Runtime OOB tests updated to span_out_of_range — good coverageChecks for front/back on empty, at(), operator[], first/last, and subspan now expect etl::span_out_of_range. Matches new error paths.
Also applies to: 473-475, 489-491, 574-576, 598-600, 624-626, 648-650, 711-724
include/etl/span.h (3)
105-117: New exception type span_out_of_rangeDedicated out-of-range exception for span is a clean separation from array exceptions.
256-270: Fixed-extent accessors/subviews: static asserts + conditional throw/assert paths
- front/back guarded by ETL_STATIC_ASSERT (Extent > 0).
- at()/operator[] now use span_out_of_range and follow the C++11 ternary-throw pattern when needed.
- first/last/subspan enforce COUNT/OFFSET at compile-time; runtime-count variants gate throw/assert and noexcept via the new macros.
Also applies to: 405-419, 425-434, 441-446, 451-460, 466-472, 477-488, 494-506, 534-548
718-741: Dynamic-extent accessors/subviews: exception/assert gating and noexcept toggling look correctRuntime bounds checks now consistently throw span_out_of_range when exceptions and EXTRA checks are enabled, otherwise assert; noexcept reflects that state. Operator[] follows the C++11 ternary-throw approach for constexpr contexts.
Also applies to: 876-889, 896-905, 911-934, 940-963, 971-985, 1010-1017
4147216
into
ETLCPP:pull-request/#1188-etl-array-checks
This add the optional assert checks to etl::span. Several of the span checks needed to throw from C++11 constexpr so the ternary operation was used where needed.
Adding macros
ETL_CHECKING_XXXandETL_NOT_CHECKING_XXXfor the optional index operator checks and extra checks. This makes the macro usage a little simpler compared to checking if the right macro is defined.Added
span_out_of_rangeexception. Fixed theatfunctions were using the wrong exception (array_out_of_range).Some of the static asserts in the static span class where unessesarily checking that
Extent != etl::dynamic_extent.