Skip to content

Conversation

@mike919192
Copy link
Contributor

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_XXX and ETL_NOT_CHECKING_XXX for 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_range exception. Fixed the at functions 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.

@mike919192
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Oct 8, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link

coderabbitai bot commented Oct 8, 2025

Walkthrough

  • Added explicit checking-state macros in include/etl/error_handler.h: ETL_CHECKING_INDEX_OPERATOR/ETL_NOT_CHECKING_INDEX_OPERATOR and ETL_CHECKING_EXTRA/ETL_NOT_CHECKING_EXTRA.
  • Updated include/etl/array.h to use ETL_NOT_USING_CPP14 and ETL_CHECKING_INDEX_OPERATOR in the operator[] const-correctness conditional.
  • Introduced etl::span_out_of_range in include/etl/span.h and replaced previous range errors with this type across at(), operator[], front(), back(), and subview methods, with static assertions and throw/assert paths gated by macros, for both fixed and dynamic extents.
  • Adjusted tests to expect etl::span_out_of_range and expanded out-of-range coverage.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.59% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title “etl::span checks” is too generic and does not clearly summarise the primary changes such as adding optional assert checks, new checking macros, and the span_out_of_range exception, making it difficult for reviewers to understand the main focus at a glance. Consider clarifying the title to explicitly mention the addition of optional assert checks, the new span_out_of_range exception, and enhancements to checking macros for etl::span to better convey the main changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed The description accurately outlines the key modifications including optional assert checks, new ETL_CHECKING_XXX macros, the span_out_of_range exception, and fixes to at() and static asserts, all of which directly correspond to the changes in the pull request.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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 good

Runtime 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 TU

Define/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

📥 Commits

Reviewing files that changed from the base of the PR and between 2c17011 and b2d0bbb.

📒 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_Case

Please 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 summaries or fix prompts for AIs
  • If the highlighted issue is complex, don't suggest a solution
  • Don't use emojis

Files:

  • include/etl/array.h
  • include/etl/error_handler.h
  • include/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 macro

Condition 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 introduced

ETL_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 coverage

Checks 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_range

Dedicated 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 correct

Runtime 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

@jwellbelove jwellbelove merged commit 4147216 into ETLCPP:pull-request/#1188-etl-array-checks Oct 13, 2025
83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants