-
-
Notifications
You must be signed in to change notification settings - Fork 471
Bugfix: array noexcept expr #1208
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
Bugfix: array noexcept expr #1208
Conversation
WalkthroughThe pull request modifies the noexcept specifications for the array subscript operator in Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 (1)
include/etl/array.h (1)
708-722
: AddETL_NOEXCEPT
tooperator[]
overloads in the zero-sized array specialisation for consistency.The
operator[]
overloads at lines 708 and 719 lack noexcept specifications, whilst the main template usesETL_NOEXCEPT_EXPR(...)
on the same methods. The entire zero-sized specialisation systematically omits noexcept specifications despite all methods (includingat()
,front()
,back()
) performing operations that cannot throw. Since these operators call*data()
without bounds checking, addingETL_NOEXCEPT
would align with the main template's approach and improve clarity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
include/etl/array.h
(2 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
orfix prompts for AIs
- If the highlighted issue is complex, don't suggest a solution
- Don't use emojis
Files:
include/etl/array.h
🧬 Code graph analysis (1)
include/etl/array.h (1)
include/etl/span.h (2)
ETL_NOEXCEPT_EXPR
(534-548)ETL_NOEXCEPT_EXPR
(1010-1017)
⏰ 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: GCC C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++23 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)
- GitHub Check: Clang C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++14 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Clang C++17 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++17 Linux - STL (ubuntu-22.04)
- GitHub Check: Windows - STL - Force C++03
- GitHub Check: Clang C++14 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Windows - No STL
- GitHub Check: Windows - No STL - Force C++03
- GitHub Check: Windows - STL
- 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++14 Linux - STL (ubuntu-22.04)
🔇 Additional comments (2)
include/etl/array.h (2)
146-151
: LGTM! Correct noexcept specification foroperator[]
.The noexcept condition now properly aligns with the
ETL_ASSERT_CHECK_INDEX_OPERATOR
macro used in the function body. The operator is correctly marked noexcept when either exceptions are disabled or index operator checking is disabled.
159-169
: LGTM! Correct noexcept specification for constoperator[]
.The noexcept condition properly matches both the
ETL_ASSERT_CHECK_INDEX_OPERATOR
assertion and the conditional exception throw in the C++11/C++14 path. The specification is consistent with the non-const overload.
Maybe due to merge error, the etl::array [] operator overloads have incorrect
ETL_NOEXCEPT_EXPR
. This corrects the expressions to use the correct ETL options.