etl array checks

[mike919192]

This is reworking the previous PR #1175 based on the discussion in that PR. In that discussion we decided that the additional checks should be enabled the same way that the existing ETL_CHECK_PUSH_POP is.

2 new check macros are added ETL_CHECK_INDEX_OPERATOR and ETL_CHECK_EXTRA. ETL_CHECK_INDEX_OPERATOR is intended for checks using the [ ] operator. ETL_CHECK_EXTRA is for any extra checks that don't fall into an existing check option.

The added checks are implemented only on etl::array. Moving forward it can be implemented on more classes in the future.

[coderabbitai]

Walkthrough

The changes add runtime bounds checks to etl::array operations. operator[] now performs index assertions and, in const builds with exceptions enabled, may throw etl::array_out_of_range. front() and back() gain static assertions to ensure non-empty arrays. Additional range and iterator validity checks are added to insert and erase variants, including erase(first, last) with stricter bounds validation. New conditional assertion macros (ETL_ASSERT_CHECK_INDEX_OPERATOR, ETL_ASSERT_CHECK_PUSH_POP, ETL_ASSERT_CHECK_EXTRA) are introduced in error_handler.h, with corresponding flags enabled in test/etl_profile.h. Tests are extended to verify out-of-range operations throw etl::array_out_of_range.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "etl array checks" is concise and directly related to the main change—adding additional runtime checks for etl::array and introducing new ETL_CHECK_* macros—so it accurately reflects the primary intent of the changeset; it is terse and could be slightly more descriptive or capitalised for readability but remains on-topic.
Description Check	✅ Passed	The pull request description directly describes the changes made: adding ETL_CHECK_INDEX_OPERATOR and ETL_CHECK_EXTRA check macros and applying additional index/extra checks to etl::array, mirroring the intent and scope shown in the code changes and tests; it also notes alignment with the existing ETL_CHECK_PUSH_POP mechanism and that the implementation is limited to etl::array for now. The description is specific to the changeset and not off-topic, and adequately explains the motivation and scope. Therefore it is related to the changeset and passes this lenient check.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

include/etl/array.h (3)

161-169: Consider simplifying the conditional compilation logic.

The special case for C++11 constexpr with exceptions could be simplified by using a helper macro or consolidating the logic.

---

461-462: Inconsistent boundary checks between insert methods.

The iterator-based insert() allows insertion at position < cend() (Line 461), which excludes the end position. This is inconsistent with typical container semantics where insertion at end() is valid.

Also applies to: 494-495

---

614-616: Documentation inconsistency in comments.

The parameter documentation refers to "first" and "last" but the previous comment block (lines 601-603) still mentions "position" and "value". This should be updated for consistency.

Apply this fix:

    //*************************************************************************
    /// Erases a range of values from the array.
-   ///\param first The first item to erase.
-   ///\param last  The one past the last item to erase.
+   ///\param first The first item to erase.
+   ///\param last  The one past the last item to erase.
    ///\param value The value to use to overwrite the last elements in the array.
    //*************************************************************************

📜 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 f9cf20d and 5b430a1.

📒 Files selected for processing (4)

• include/etl/array.h (17 hunks)
• include/etl/error_handler.h (1 hunks)
• test/etl_profile.h (1 hunks)
• test/test_array.cpp (6 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/error_handler.h
• include/etl/array.h

🧠 Learnings (3)

📓 Common learnings

Learnt from: filipe-cuim
PR: ETLCPP/etl#1174
File: include/etl/type_traits.h:1974-1985
Timestamp: 2025-09-01T14:17:35.361Z
Learning: The current implementation in this PR correctly uses only individual trait flags (like ETL_USING_BUILTIN_IS_ASSIGNABLE) in preprocessor guards, not the global ETL_USE_TYPE_TRAITS_BUILTINS flag, because the global flag automatically defines all individual flags when enabled, making dual checks redundant.

📚 Learning: 2025-09-01T14:17:35.361Z

Learnt from: filipe-cuim
PR: ETLCPP/etl#1174
File: include/etl/type_traits.h:1974-1985
Timestamp: 2025-09-01T14:17:35.361Z
Learning: The current implementation in this PR correctly uses only individual trait flags (like ETL_USING_BUILTIN_IS_ASSIGNABLE) in preprocessor guards, not the global ETL_USE_TYPE_TRAITS_BUILTINS flag, because the global flag automatically defines all individual flags when enabled, making dual checks redundant.

Applied to files:

• include/etl/error_handler.h
• test/etl_profile.h

📚 Learning: 2025-09-01T14:16:52.836Z

Learnt from: filipe-cuim
PR: ETLCPP/etl#1174
File: include/etl/type_traits.h:1974-1980
Timestamp: 2025-09-01T14:16:52.836Z
Learning: In the ETL library, ETL_USE_TYPE_TRAITS_BUILTINS is a global flag that automatically defines all individual ETL_USING_BUILTIN_IS_* flags (like ETL_USING_BUILTIN_IS_ASSIGNABLE, ETL_USING_BUILTIN_IS_CONSTRUCTIBLE, etc.) in include/etl/profiles/determine_builtin_support.h. Therefore, the per-trait preprocessor guards only need to check their individual flags, not both the individual and global flags.

Applied to files:

• test/etl_profile.h

🧬 Code graph analysis (1)

test/test_array.cpp (1)

include/etl/array.h (1)

• array_out_of_range (72-80)

⏰ 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: Windows - STL
• GitHub Check: GCC C++11 Linux - STL (ubuntu-22.04)
• GitHub Check: Clang C++17 Linux - STL (ubuntu-22.04)
• GitHub Check: Clang C++23 Linux - No STL - Force C++03 (ubuntu-22.04)
• GitHub Check: Clang C++17 Linux - No STL (ubuntu-22.04)
• GitHub Check: GCC C++11 Linux - No STL (ubuntu-22.04)
• GitHub Check: Clang C++23 OSX - No STL (macos-13)
• GitHub Check: Clang C++23 OSX - STL (macos-13)
• GitHub Check: Clang C++11 Linux - STL (ubuntu-22.04)
• GitHub Check: Clang C++23 OSX - STL - Force C++03 (macos-13)
• GitHub Check: Clang C++23 OSX - No STL - Force C++03 (macos-13)
• GitHub Check: Clang C++20 OSX - No STL (macos-13)
• GitHub Check: Clang C++20 Linux - No STL - Force C++03 (ubuntu-22.04)
• GitHub Check: Clang C++20 Linux - STL - Force C++03 (ubuntu-22.04)
• GitHub Check: GCC C++20 Linux - STL (ubuntu-22.04)
• GitHub Check: GCC C++20 Linux - No STL (ubuntu-22.04)
• GitHub Check: Clang C++20 Linux - No STL (ubuntu-22.04)
• GitHub Check: GCC C++20 Linux - No STL - Force C++03 (ubuntu-22.04)
• GitHub Check: Clang C++23 Linux - No STL (ubuntu-22.04)
• GitHub Check: Clang C++23 Linux - STL - Force C++03 (ubuntu-22.04)

🔇 Additional comments (12)

test/etl_profile.h (1)

37-38: LGTM! New check macros added correctly.

The new macros ETL_CHECK_INDEX_OPERATOR and ETL_CHECK_EXTRA are properly defined after ETL_CHECK_PUSH_POP, maintaining consistency with the existing pattern.

include/etl/error_handler.h (1)

367-383: LGTM! Conditional assertion macros implemented correctly.

The new conditional assertion macros follow the established pattern, properly forwarding to ETL_ASSERT when their corresponding check flags are defined, and expanding to nothing otherwise.

test/test_array.cpp (5)

138-139: LGTM! Out-of-bounds checks added for operator[].

The tests correctly verify that accessing data[data.size()] throws etl::array_out_of_range when checks are enabled.

Also applies to: 151-152

---

451-453: LGTM! Boundary check for insert_at added.

The test properly verifies that inserting at data.size() throws an exception.

---

504-506: LGTM! Out-of-range check for insert_at with range added.

The test correctly validates that inserting a range at an invalid position throws an exception.

---

561-563: LGTM! Boundary check for erase_at added.

The test properly verifies that erasing at data.size() throws an exception.

---

618-623: LGTM! Comprehensive range validation for erase_range added.

The tests correctly check both invalid range conditions: when first > last and when the range extends beyond the array bounds.

include/etl/array.h (5)

148-151: LGTM! Index bounds check added for non-const operator[].

The check properly validates the index before accessing the buffer.

---

178-179: LGTM! Static assertions prevent misuse of empty arrays.

The static assertions properly guard against calling front() and back() on empty arrays at compile time.

Also applies to: 189-190, 201-202, 212-213

---

522-523: LGTM! Comprehensive bounds validation for erase operations.

All erase operations properly validate their input ranges and positions.

Also applies to: 534-535, 549-551, 563-564

---

577-578: LGTM! Erase operations with value parameter properly validated.

The bounds checks are correctly implemented for all erase variants that accept a value parameter.

Also applies to: 589-590, 607-608, 620-621

---

449-450: No change required — insert_at prohibits insertion at end by design.

Tests (test/test_array.cpp:452, 505) assert that insert_at(data.size(), ...) throws etl::array_out_of_range, so the position < SIZE check is intentional. Change only if you also update tests/documentation.

[mike919192]

@jwellbelove Just FYI, the branch that this was merged into still contains the ETL_DEBUG_ASSERT macros. I was fine with removing those if we were not going to use that approach.