Removed forced unsigned int cast in type_def bit-shift operators #1178
+45
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughMember and non‑member shift operators in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant P as Primitive (int/unsigned/...)
participant D as etl::type_def
Note over P,D #D3E4CD: New templated overloads enable shifts with either operand order
P->>D: operator<<(P lhs, D rhs) %% templated friend returning P when allowed
D-->>P: returns shifted P
D->>D: operator<<(D lhs, P rhs) / operator<<=(P rhs) %% member/friend templated
P->>D: operator>>(P lhs, D rhs) %% templated friend returning P when allowed
D-->>P: returns shifted P
D->>D: operator>>(D lhs, P rhs) / operator>>=(P rhs) %% member/friend templated
Note over P,D #FCE5C9: SFINAE via etl::is_integral<T> (ETL_USING_CPP11)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
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.
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. ✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
include/etl/type_def.h (4)
293-297: Good fix; prevent accidental signed calls by deleting signed overloads.Switching rhs to unsigned removes negative shift types, but implicit conversion from signed actuals can still sneak through. Delete common signed overloads to fail fast at compile-time.
Apply this diff to add deleted overloads next to the unsigned version:
ETL_CONSTEXPR14 type_def& operator <<=(unsigned int rhs) ETL_NOEXCEPT { value <<= rhs; return *this; } +#if ETL_USING_CPP11 + // Disallow accidental signed arguments (e.g. -1) from binding. + type_def& operator<<=(int) = delete; + type_def& operator<<=(long) = delete; + type_def& operator<<=(long long) = delete; +#endifOptional: consider using size_t for rhs to mirror std::bitset and avoid narrowing on platforms where size_t > unsigned int.
300-304: Mirror the signed-deletion for >>= to close the gap.Same rationale as <<=; add deleted overloads to stop accidental negatives via implicit conversion.
ETL_CONSTEXPR14 type_def& operator >>=(unsigned int rhs) ETL_NOEXCEPT { value >>= rhs; return *this; } +#if ETL_USING_CPP11 + type_def& operator>>=(int) = delete; + type_def& operator>>=(long) = delete; + type_def& operator>>=(long long) = delete; +#endif
592-595: Non-member <<: add deleted signed overloads to force explicit intent.Prevents calls like lhs << -1 from silently converting to a huge unsigned and invoking UB (shift ≥ width).
friend ETL_CONSTEXPR type_def operator <<(const type_def& lhs, unsigned int rhs) ETL_NOEXCEPT { return type_def(lhs.value << rhs); } +#if ETL_USING_CPP11 + friend ETL_CONSTEXPR type_def operator<<(const type_def&, int) = delete; + friend ETL_CONSTEXPR type_def operator<<(const type_def&, long) = delete; + friend ETL_CONSTEXPR type_def operator<<(const type_def&, long long) = delete; +#endif
600-603: Non-member >>: mirror the deleted signed overloads.Keeps the interface consistent and stops accidental negatives.
friend ETL_CONSTEXPR type_def operator >>(const type_def& lhs, unsigned int rhs) ETL_NOEXCEPT { return type_def(lhs.value >> rhs); } +#if ETL_USING_CPP11 + friend ETL_CONSTEXPR type_def operator>>(const type_def&, int) = delete; + friend ETL_CONSTEXPR type_def operator>>(const type_def&, long) = delete; + friend ETL_CONSTEXPR type_def operator>>(const type_def&, long long) = delete; +#endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
include/etl/type_def.h(2 hunks)
⏰ 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++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: Clang C++20 OSX - No STL (macos-13)
- GitHub Check: GCC C++20 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - 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: 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 - No STL (ubuntu-22.04)
- GitHub Check: GCC C++14 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++14 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++23 OSX - No STL (macos-13)
- GitHub Check: Clang C++23 Linux - No STL (ubuntu-22.04)
- GitHub Check: Windows - STL
- GitHub Check: Clang C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - No STL (ubuntu-22.04)
- GitHub Check: Windows - No STL
- GitHub Check: GCC C++17 Linux - No STL (ubuntu-22.04)
🔇 Additional comments (1)
include/etl/type_def.h (1)
293-304: Ensurerhs < bit-widthto prevent undefined behaviour
rhs ≥ sizeof(TValue) * CHAR_BITstill yields UB by the standard; document this precondition or add a debug-onlyETL_ASSERT(rhs < sizeof(TValue) * CHAR_BIT)in non-constexpr contexts- No negative-literal shift usages found via
rg -nP '(<<|>>)\s*-\s*\d+'/'(<<=|>>=)\s*-\s*\d+', but dynamic or oversizedrhsvalues remain unchecked
|
I would have included this in today's release, but I can't until the failing unit tests are fixed. |
…r shifts This allows the library user to explicitly use unsigned values to avoid UB
|
I've fixed the tests, and I've also made the |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
test/test_type_def.cpp (1)
200-204: Prefer constexpr and width‑appropriate literals for new operands.Make these constants constexpr and use U (not UL) to match uint32_t across ILP32/LP64.
Apply:
- uint32_t i3 = 3UL; + constexpr uint32_t i3 = 3U; ... - type_t t3(3UL); + constexpr type_t t3(3U);include/etl/type_def.h (1)
618-630: Preserve the promoted result type for T op type_def.Returning T can narrow (e.g., unsigned char << 3 yields int). Prefer decltype(lhs << rhs.value) on C++11+.
Apply:
- typename etl::enable_if<etl::is_arithmetic<T>::value, T>::type + typename etl::enable_if<etl::is_integral<T>::value, decltype(T() << rhs.value)>::typeand
- typename etl::enable_if<etl::is_arithmetic<T>::value, T>::type + typename etl::enable_if<etl::is_integral<T>::value, decltype(T() >> rhs.value)>::type(For pre‑C++11 path keep the existing fallback.)
Also applies to: 646-658
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
include/etl/type_def.h(2 hunks)test/test_type_def.cpp(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 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.
⏰ 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 OSX - No STL (macos-13)
- GitHub Check: Clang C++23 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++23 OSX - STL (macos-13)
- GitHub Check: Clang C++23 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC 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++20 Linux - STL (ubuntu-22.04)
- GitHub Check: Windows - STL - Force C++03
- GitHub Check: GCC C++23 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++20 OSX - STL (macos-13)
- GitHub Check: Windows - STL
- GitHub Check: GCC C++23 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++14 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++14 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++14 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++14 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++11 Linux - STL (ubuntu-22.04)
🔇 Additional comments (2)
test/test_type_def.cpp (1)
182-185: In‑place shift tests look good.These mirror the new compound operators correctly.
include/etl/type_def.h (1)
293-305: Optional: define behaviour for out‑of‑range shift counts or document as precondition.Even with negative counts avoided, rhs ≥ numeric_limits::digits remains UB. Either clamp/mask or document the precondition.
Would you like me to add a guarded implementation (clamp to width or mask) behind an ETL_CONFIG flag?
|
There are still some C++11 issues. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
include/etl/type_def.h (4)
293-300: Shift count must be integral and non‑negative; current gating still permits UB
is_arithmetic<T>allows floating types and does not prevent negative counts. That defeats the PR goal of eliminating UB and can also cause hard compile errors when instantiated with floats.Apply at least the constraint fix:
-#if ETL_USING_CPP11 - typename etl::enable_if<etl::is_arithmetic<T>::value, type_def&>::type +#if ETL_USING_CPP11 + typename etl::enable_if<etl::is_integral<T>::value, type_def&>::typeAnd (outside the shown range) consider guarding and casting the count:
- value <<= rhs; + // Optional debug guard (only if your assert macro is available): + // ETL_ASSERT(rhs >= 0); + value <<= static_cast<typename etl::make_unsigned<T>::type>(rhs);
307-314: Same issue for >>= compound shift‑assignMirror the
<<=fixes: restrict to integrals and treat the count as unsigned to avoid UB from negatives.-#if ETL_USING_CPP11 - typename etl::enable_if<etl::is_arithmetic<T>::value, type_def&>::type +#if ETL_USING_CPP11 + typename etl::enable_if<etl::is_integral<T>::value, type_def&>::typeBody tweak (outside the range):
- value >>= rhs; + // ETL_ASSERT(rhs >= 0); + value >>= static_cast<typename etl::make_unsigned<T>::type>(rhs);
606-617: Constrain RHS to integral types (floats will hard‑error on shift)Using
is_arithmetic<T>here allowsfloat/double, which will instantiate and then fail in the body. Restrict tois_integral<T>.-#if ETL_USING_CPP11 - typename etl::enable_if<etl::is_arithmetic<T>::value, type_def>::type +#if ETL_USING_CPP11 + typename etl::enable_if<etl::is_integral<T>::value, type_def>::typeAlso consider casting the shift count to an unsigned type in the body (outside this hunk):
- return type_def(lhs.value << rhs); + // ETL_ASSERT(rhs >= 0); + return type_def(lhs.value << static_cast<typename etl::make_unsigned<T>::type>(rhs));
634-645: Apply the same integral‑only constraint and unsigned count handling for >>Restrict to integrals and avoid negative counts; otherwise UB persists.
-#if ETL_USING_CPP11 - typename etl::enable_if<etl::is_arithmetic<T>::value, type_def>::type +#if ETL_USING_CPP11 + typename etl::enable_if<etl::is_integral<T>::value, type_def>::typeBody (outside the hunk):
- return type_def(lhs.value >> rhs); + // ETL_ASSERT(rhs >= 0); + return type_def(lhs.value >> static_cast<typename etl::make_unsigned<T>::type>(rhs));
🧹 Nitpick comments (1)
include/etl/type_def.h (1)
618-630: Return type ‘T’ deviates from usual integer promotions; please confirm intent
operator<<(T lhs, type_def)returnsT, whereas built‑ins return a promoted type (oftenint). This may truncate results and changeautodeduced types.Options:
- Return
decltype(lhs << rhs.value)in C++11+ (falls back toTpre‑C++11 if needed).- Or clearly document the truncating behaviour.
Also apply the integral constraint to avoid float instantiation:
-#if ETL_USING_CPP11 - typename etl::enable_if<etl::is_arithmetic<T>::value, T>::type +#if ETL_USING_CPP11 + typename etl::enable_if<etl::is_integral<T>::value, T>::typeAnd treat the count as unsigned in the body (outside this hunk):
- return lhs << rhs.value; + // ETL_ASSERT(rhs.value >= 0); + return lhs << static_cast<typename etl::make_unsigned<TValue>::type>(rhs.value);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
include/etl/type_def.h(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 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.
⏰ 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++20 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++20 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++23 OSX - STL (macos-13)
- GitHub Check: GCC C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++14 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++17 Linux - STL (ubuntu-22.04)
- GitHub Check: Windows - STL
- GitHub Check: Windows - STL - Force C++03
- GitHub Check: Windows - No STL
- GitHub Check: GCC C++23 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Clang C++14 Linux - No STL (ubuntu-22.04)
- GitHub Check: Windows - No STL - Force C++03
- GitHub Check: GCC C++17 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++17 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC C++14 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++17 Linux - No STL (ubuntu-22.04)
Comment on lines
646
to
658
| //********************************************************************* | ||
| template <typename T> | ||
| friend ETL_CONSTEXPR | ||
| #if ETL_USING_CPP11 | ||
| typename etl::enable_if<etl::is_arithmetic<T>::value, T>::type | ||
| #else | ||
| T | ||
| #endif | ||
| operator >>(T lhs, const type_def& rhs) ETL_NOEXCEPT | ||
| { | ||
| return lhs >> rhs.value; | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Symmetry with ‘T >> type_def’: promotions and negative counts
- Consider returning
decltype(lhs >> rhs.value)to match built‑ins (C++11+), or document truncation toT. - Constrain
Tto integrals and cast the count to unsigned.
-#if ETL_USING_CPP11
- typename etl::enable_if<etl::is_arithmetic<T>::value, T>::type
+#if ETL_USING_CPP11
+ typename etl::enable_if<etl::is_integral<T>::value, T>::typeBody (outside the hunk):
- return lhs >> rhs.value;
+ // ETL_ASSERT(rhs.value >= 0);
+ return lhs >> static_cast<typename etl::make_unsigned<TValue>::type>(rhs.value);🏁 Script executed:
#!/bin/bash
# Locate the definition of type_def to inspect TValue and member 'value'
rg -nP '^(template\s*<[^>]+>\s*)?(class|struct)\s+type_def\b' -C5 include/etl/type_def.h
# Search for any alias or typedef of TValue within the same file
rg -nP 'using\s+TValue' -n include/etl/type_def.h
rg -nP 'typedef\s+.*TValue' -n include/etl/type_def.h
# Show the operator>> implementation context around the original snippet
sed -n '640,680p' include/etl/type_def.hLength of output: 1215
Restrict operator>>(T, type_def) to integral T and guard against negative counts
- Change the enable_if to
etl::is_integral<T>::valueinstead ofis_arithmetic<T> - Return
decltype(lhs >> rhs.value)(C++11+) or document that returningTtruncates the promoted result - Insert
ETL_ASSERT(rhs.value >= 0);and cast the shift count to an unsigned type (e.g.
static_cast<typename etl::make_unsigned<TValue>::type>(rhs.value)) to avoid undefined behaviour on negative values
🤖 Prompt for AI Agents
In include/etl/type_def.h around lines 646 to 658, the binary right-shift
operator is too permissive and can UB on negative shift counts: change the
enable_if condition from etl::is_arithmetic<T>::value to
etl::is_integral<T>::value so only integral lhs types are allowed; for C++11
builds change the return type to decltype(lhs >> rhs.value) (guarded by
ETL_USING_CPP11) so the promoted result type is preserved (otherwise leave the
existing T return but add a note); add ETL_ASSERT(rhs.value >= 0); and cast the
shift count to an unsigned type when invoking the builtin shift, e.g.
static_cast<typename etl::make_unsigned<TValue>::type>(rhs.value), to avoid
undefined behaviour from negative shift counts.
…quire integral operands
ockeydockey
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
test/test_type_def.cpp (1)
234-244: Previous UB concerns have been addressed.The shift operations now use bounded shift counts (
i3with value 3) instead of the previously problematic large values. This eliminates the undefined behaviour that was flagged in earlier reviews.include/etl/type_def.h (4)
293-304: LGTM! Proper template constraints and constexpr usage.The left-shift assignment operator is correctly templated with
is_integral<T>constraints and usesETL_CONSTEXPR14as appropriate for mutating operations.
307-318: LGTM! Consistent implementation for right-shift assignment.The right-shift assignment operator follows the same correct pattern as the left-shift variant with proper integral constraints.
606-616: LGTM! Proper integral constraints for shift operators.The shift operator correctly uses
is_integral<T>instead ofis_arithmetic<T>, preventing ill-formed instantiations with floating-point types.
634-644: LGTM! Consistent right-shift implementation.The right-shift operator properly mirrors the left-shift constraints with integral type requirements.
🧹 Nitpick comments (1)
include/etl/type_def.h (1)
646-657: Well-implemented symmetric right-shift operator.This completes the shift operator family by enabling
T >> type_defexpressions with proper integral constraints on both operands.However, consider adding runtime checks for negative shift counts to prevent undefined behaviour:
template <typename T> friend ETL_CONSTEXPR #if ETL_USING_CPP11 typename etl::enable_if<(etl::is_integral<T>::value && etl::is_integral<TValue>::value), T>::type #else T #endif operator >>(T lhs, const type_def& rhs) ETL_NOEXCEPT { // ETL_ASSERT(rhs.value >= 0); return lhs >> rhs.value; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
include/etl/type_def.h(2 hunks)test/test_type_def.cpp(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 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.
⏰ 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++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++14 Linux - STL (ubuntu-22.04)
- GitHub Check: Windows - No STL - Force C++03
- 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 - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Clang C++14 Linux - No STL (ubuntu-22.04)
- GitHub Check: Windows - STL
- GitHub Check: Windows - No STL
- GitHub Check: Clang C++20 OSX - STL - Force C++03 (macos-13)
- GitHub Check: Clang C++20 OSX - No STL - Force C++03 (macos-13)
- GitHub Check: Clang C++20 OSX - No STL (macos-13)
- GitHub Check: Clang C++20 OSX - STL (macos-13)
- GitHub Check: Clang C++20 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++20 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Windows - STL - Force C++03
- GitHub Check: GCC C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++14 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++17 Linux - STL (ubuntu-22.04)
🔇 Additional comments (3)
test/test_type_def.cpp (2)
182-186: LGTM! Shift operations properly tested.The in-place shift operators are correctly tested with both signed and unsigned shift counts.
200-203: Good addition of test variables.Adding
i3andt3with value 3 provides appropriate bounded shift counts for testing the new templated shift operators.include/etl/type_def.h (1)
618-629: Excellent addition of left-hand side shift operator.This new templated operator enables expressions like
2 << type_def_value, providing complete arithmetic symmetry. The dual constraint on bothTandTValuebeing integral is appropriate for shift operations.
|
Thank you for your patience through this process. I have addressed all of the failing tests, and have also refined the implementation. Let me know if this needs anything else. |
|
I'll try to go over the PR today or tomorrow. |
jwellbelove
reviewed
Sep 11, 2025
jwellbelove
reviewed
Sep 11, 2025
jwellbelove
reviewed
Sep 11, 2025
include/etl/type_def.h
Outdated
| //********************************************************************* | ||
| ETL_CONSTEXPR14 type_def& operator >>=(int rhs) ETL_NOEXCEPT | ||
| template <typename T> | ||
| ETL_CONSTEXPR14 |
There was a problem hiding this comment.
etl::is_integral<T> is valid for C++03
jwellbelove
reviewed
Sep 11, 2025
jwellbelove
reviewed
Sep 11, 2025
jwellbelove
reviewed
Sep 11, 2025
jwellbelove
reviewed
Sep 11, 2025
jwellbelove
reviewed
Sep 11, 2025
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
include/etl/type_def.h (1)
302-308: Avoid UB: normalise and cast the shift count in operator>>=.Same issue as <<=.
- value >>= rhs; + value >>= (static_cast<typename etl::make_unsigned<T>::type>(rhs) + % (sizeof(TValue) * CHAR_BIT));
♻️ Duplicate comments (2)
include/etl/type_def.h (2)
621-629: Guard ‘T >> type_def’ against negative/oversize counts.This was raised earlier; still applicable. Normalise the count to avoid UB.
- typename etl::enable_if<(etl::is_integral<T>::value && etl::is_integral<TValue>::value), T>::type + typename etl::enable_if<(etl::is_integral<T>::value && etl::is_integral<TValue>::value), T>::type operator >>(T lhs, const type_def& rhs) ETL_NOEXCEPT { - return lhs >> rhs.value; + return lhs >> (static_cast<typename etl::make_unsigned<TValue>::type>(rhs.value) + % (sizeof(TValue) * CHAR_BIT)); }
603-610: Guard ‘T << type_def’ against negative/oversize counts.Cast rhs.value to an unsigned count and modulo by width to avoid UB.
- friend ETL_CONSTEXPR typename etl::enable_if<(etl::is_integral<T>::value && etl::is_integral<TValue>::value), T>::type + friend ETL_CONSTEXPR typename etl::enable_if<(etl::is_integral<T>::value && etl::is_integral<TValue>::value), T>::type operator <<(T lhs, const type_def& rhs) ETL_NOEXCEPT { - return lhs << rhs.value; + return lhs << (static_cast<typename etl::make_unsigned<TValue>::type>(rhs.value) + % (sizeof(TValue) * CHAR_BIT)); }
🧹 Nitpick comments (2)
include/etl/type_def.h (2)
596-610: Optional: add debug-time checks for negative counts.If you want to surface misuse early, add a lightweight debug assert (when available) that rhs >= 0 before the cast in signed overloads. Happy to wire this to ETL’s assert macro if you confirm the preferred form.
Also applies to: 614-629
293-305: Request: extend tests to cover edge cases.Please add cases for:
- negative signed counts (expect debug assert/failure or well-defined masked result),
- counts ≥ bit-width,
- both signed/unsigned T and TValue combinations.
I can draft these if useful.
Also applies to: 596-629
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
include/etl/type_def.h(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 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/type_def.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++14 Linux - STL (ubuntu-22.04)
- GitHub Check: Windows - STL - Force C++03
- GitHub Check: Windows - STL
- GitHub Check: GCC C++14 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC 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 - STL (ubuntu-22.04)
- GitHub Check: Windows - No STL
- GitHub Check: GCC C++23 Linux - STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Clang C++23 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++23 OSX - STL (macos-13)
- GitHub Check: Clang C++23 OSX - STL - Force C++03 (macos-13)
- GitHub Check: GCC C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++23 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++17 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++17 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++23 Linux - STL - Force C++03 (ubuntu-22.04)
Comment on lines
+293
to
299
| template <typename T> | ||
| ETL_CONSTEXPR14 typename etl::enable_if<etl::is_integral<T>::value, type_def&>::type | ||
| operator <<=(T rhs) ETL_NOEXCEPT | ||
| { | ||
| value <<= rhs; | ||
| return *this; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid UB: normalise and cast the shift count in operator<<=.
Negative or oversized counts are still UB. Cast to an unsigned type and mask/modulo by the bit-width.
Apply:
- value <<= rhs;
+ value <<= (static_cast<typename etl::make_unsigned<T>::type>(rhs)
+ % (sizeof(TValue) * CHAR_BIT));If CHAR_BIT isn’t already available via includes, please add:
#include <climits> // for CHAR_BIT🤖 Prompt for AI Agents
In include/etl/type_def.h around lines 293-299, the operator<<= currently shifts
by a possibly negative or oversized rhs which is UB; compute an unsigned
normalized shift count first (e.g. auto bits = sizeof(value) * CHAR_BIT; auto
shift = static_cast<unsigned>(rhs) % bits; or use bitmasking with (bits - 1) for
power-of-two widths), then perform value <<= shift; and add #include <climits>
at the top if CHAR_BIT isn't already available.
Comment on lines
+596
to
602
| template <typename T> | ||
| friend ETL_CONSTEXPR typename etl::enable_if<etl::is_integral<T>::value, type_def>::type | ||
| operator <<(const type_def& lhs, T rhs) ETL_NOEXCEPT | ||
| { | ||
| return type_def(lhs.value << rhs); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Constrain to integral TValue and guard shift count.
- Gate on integral TValue to avoid hard errors when TValue is non‑integral.
- Normalise the count to avoid UB.
- friend ETL_CONSTEXPR typename etl::enable_if<etl::is_integral<T>::value, type_def>::type
+ friend ETL_CONSTEXPR typename etl::enable_if<(etl::is_integral<T>::value && etl::is_integral<TValue>::value), type_def>::type
operator <<(const type_def& lhs, T rhs) ETL_NOEXCEPT
{
- return type_def(lhs.value << rhs);
+ return type_def(lhs.value
+ << (static_cast<typename etl::make_unsigned<T>::type>(rhs)
+ % (sizeof(TValue) * CHAR_BIT)));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| template <typename T> | |
| friend ETL_CONSTEXPR typename etl::enable_if<etl::is_integral<T>::value, type_def>::type | |
| operator <<(const type_def& lhs, T rhs) ETL_NOEXCEPT | |
| { | |
| return type_def(lhs.value << rhs); | |
| } | |
| template <typename T> | |
| friend ETL_CONSTEXPR typename etl::enable_if<(etl::is_integral<T>::value && etl::is_integral<TValue>::value), type_def>::type | |
| operator <<(const type_def& lhs, T rhs) ETL_NOEXCEPT | |
| { | |
| return type_def(lhs.value | |
| << (static_cast<typename etl::make_unsigned<T>::type>(rhs) | |
| % (sizeof(TValue) * CHAR_BIT))); | |
| } |
Comment on lines
+614
to
619
| template <typename T> | ||
| friend ETL_CONSTEXPR typename etl::enable_if<etl::is_integral<T>::value, type_def>::type | ||
| operator >>(const type_def& lhs, T rhs) ETL_NOEXCEPT | ||
| { | ||
| return type_def(lhs.value >> rhs); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Constrain to integral TValue and guard shift count.
Mirror the <<= fix for ‘type_def >> T’.
- friend ETL_CONSTEXPR typename etl::enable_if<etl::is_integral<T>::value, type_def>::type
+ friend ETL_CONSTEXPR typename etl::enable_if<(etl::is_integral<T>::value && etl::is_integral<TValue>::value), type_def>::type
operator >>(const type_def& lhs, T rhs) ETL_NOEXCEPT
{
- return type_def(lhs.value >> rhs);
+ return type_def(lhs.value
+ >> (static_cast<typename etl::make_unsigned<T>::type>(rhs)
+ % (sizeof(TValue) * CHAR_BIT)));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| template <typename T> | |
| friend ETL_CONSTEXPR typename etl::enable_if<etl::is_integral<T>::value, type_def>::type | |
| operator >>(const type_def& lhs, T rhs) ETL_NOEXCEPT | |
| { | |
| return type_def(lhs.value >> rhs); | |
| } | |
| template <typename T> | |
| friend ETL_CONSTEXPR typename etl::enable_if<(etl::is_integral<T>::value && etl::is_integral<TValue>::value), type_def>::type | |
| operator >>(const type_def& lhs, T rhs) ETL_NOEXCEPT | |
| { | |
| return type_def(lhs.value | |
| >> (static_cast<typename etl::make_unsigned<T>::type>(rhs) | |
| % (sizeof(TValue) * CHAR_BIT))); | |
| } |
🤖 Prompt for AI Agents
In include/etl/type_def.h around lines 614 to 619, the operator>> template
currently only constrains T to be integral and performs an unguarded shift;
update the enable_if to require both T and the underlying TValue of type_def be
integral, and add a guard against out-of-range shift counts: convert rhs to an
unsigned type (use etl::make_unsigned_t<T>), compute the bit-width of TValue
(e.g. sizeof(TValue) * CHAR_BIT), if the unsigned shift is >= width return a
zero-initialized type_def, otherwise perform the right shift with the
masked/checked shift count; keep the function noexcept and return
type_def(lhs.value >> shift).
jwellbelove
changed the base branch from
master
to
pull-request/#1178-Removed-forced-unsigned-int-cast-in-type_def-bit-shift-operators
jwellbelove
merged commit 603c977
into
ETLCPP:pull-request/#1178-Removed-forced-unsigned-int-cast-in-type_def-bit-shift-operators
83 checks passed
jwellbelove
pushed a commit
that referenced
this pull request
Sep 11, 2025
* Removed UB in type_def bit-shift operators * Changed shift operators to allow both signed and unsigned operands for shifts This allows the library user to explicitly use unsigned values to avoid UB * Fixed constexpr errors for CPP11 * Changed is_arithmetic checks to use is_integral since valid shifts require integral operands * Removed need for CPP11 since changes are CPP03 compatible
jwellbelove
added a commit
that referenced
this pull request
Sep 23, 2025
* Regression fix: Support zero arguments emplace() in etl::optional (#1183) * Added coderabbitai configuration * Added builtin mem function tests * Modified etl::typed_storage * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Added ETL_NOEXCEPT and ETL_NOEXCEPT_IF_NO_THROW * Added etl::typed_storage_ext and swap for same * Added etl::typed_storage_ext and swap for same # Conflicts: # include/etl/alignment.h * Added release notes * Fixes to GCC -O2 errors * Changed char* parameters to value_type* parameters * Fixed compilation issues for const containers unit tests * Added automatic selection of __builtin_memxxx functions for GCC and clang * Added enhanced coderabbit configuration * Updated version and release notes * Disabled constexpr const container tests for C++11 * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Updated version and release notes * feat: removed unreachable break statements (#1169) * Updated version and release notes * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Support zero arguments emplace() in etl::optional For non-fundamental types, a recent change in etl::optional was introduced that doesn't support zero arguments emplace() anymore. This change fixes it and adds the respective test. --------- Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: Drew Rife <[email protected]> * Fix etl::typed_storage by supporting omitted destructors (#1182) * Added coderabbitai configuration * Added builtin mem function tests * Modified etl::typed_storage * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Added ETL_NOEXCEPT and ETL_NOEXCEPT_IF_NO_THROW * Added etl::typed_storage_ext and swap for same * Added etl::typed_storage_ext and swap for same # Conflicts: # include/etl/alignment.h * Added release notes * Fixes to GCC -O2 errors * Changed char* parameters to value_type* parameters * Fixed compilation issues for const containers unit tests * Added automatic selection of __builtin_memxxx functions for GCC and clang * Added enhanced coderabbit configuration * Updated version and release notes * Disabled constexpr const container tests for C++11 * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Updated version and release notes * feat: removed unreachable break statements (#1169) * Updated version and release notes * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Fix etl::typed_storage by supporting omitted destructors In a recent change to alignment.h, the etl::typed_storage was changed in a way that in case of an already constructed object, the object is created via assignment. However, this contradicts the original use case that led to etl::typed_storage in the first place: #1023 The goal is to omit destructors (and at the same time support classes with deleted assignment operators), so they can be optimized out at link time. This change reverts commit ac7b268 to restore the original functionality and changes the test to reflect the original use case. * Fix missing create() in non-C++11 typed_storage_ext constructor * Typo fix --------- Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: Drew Rife <[email protected]> Co-authored-by: John Wellbelove <[email protected]> * removed navis file from project * Updated version and release notes * Removed forced unsigned int cast in type_def bit-shift operators (#1178) * Removed UB in type_def bit-shift operators * Changed shift operators to allow both signed and unsigned operands for shifts This allows the library user to explicitly use unsigned values to avoid UB * Fixed constexpr errors for CPP11 * Changed is_arithmetic checks to use is_integral since valid shifts require integral operands * Removed need for CPP11 since changes are CPP03 compatible * Delete project navis files * Add force CI check on piull requests * Removed ETL_NOEXCEPT from delegate operator(), call_if(), and call_or() Removed ETL_NOEXCEPT from closureoperator(), call_if(), and call_or() * Updated version and release notes * Updated version and release notes * Remove noexcept from delegate method stubs. (#1185) In addition to removing noexcept from call_if, this is also needed to prevent an abort when cancelling a pthread that is executing a delegate. * Updated version and release notes * Re architect the extra checks * Add CHECK_EXTRA * Fix newline at end of file * The check index macro also needs to be defined to throw * Remove ETL_VERBOSE_ERRORS macros --------- Co-authored-by: Roland Reichwein <[email protected]> Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: Drew Rife <[email protected]> Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: David Ockey <[email protected]> Co-authored-by: Marco Nilsson <[email protected]>
jwellbelove
added a commit
that referenced
this pull request
Sep 27, 2025
* Regression fix: Support zero arguments emplace() in etl::optional (#1183) * Added coderabbitai configuration * Added builtin mem function tests * Modified etl::typed_storage * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Added ETL_NOEXCEPT and ETL_NOEXCEPT_IF_NO_THROW * Added etl::typed_storage_ext and swap for same * Added etl::typed_storage_ext and swap for same # Conflicts: # include/etl/alignment.h * Added release notes * Fixes to GCC -O2 errors * Changed char* parameters to value_type* parameters * Fixed compilation issues for const containers unit tests * Added automatic selection of __builtin_memxxx functions for GCC and clang * Added enhanced coderabbit configuration * Updated version and release notes * Disabled constexpr const container tests for C++11 * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Updated version and release notes * feat: removed unreachable break statements (#1169) * Updated version and release notes * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Support zero arguments emplace() in etl::optional For non-fundamental types, a recent change in etl::optional was introduced that doesn't support zero arguments emplace() anymore. This change fixes it and adds the respective test. --------- Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: Drew Rife <[email protected]> * Fix etl::typed_storage by supporting omitted destructors (#1182) * Added coderabbitai configuration * Added builtin mem function tests * Modified etl::typed_storage * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Added ETL_NOEXCEPT and ETL_NOEXCEPT_IF_NO_THROW * Added etl::typed_storage_ext and swap for same * Added etl::typed_storage_ext and swap for same # Conflicts: # include/etl/alignment.h * Added release notes * Fixes to GCC -O2 errors * Changed char* parameters to value_type* parameters * Fixed compilation issues for const containers unit tests * Added automatic selection of __builtin_memxxx functions for GCC and clang * Added enhanced coderabbit configuration * Updated version and release notes * Disabled constexpr const container tests for C++11 * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Updated version and release notes * feat: removed unreachable break statements (#1169) * Updated version and release notes * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Fix etl::typed_storage by supporting omitted destructors In a recent change to alignment.h, the etl::typed_storage was changed in a way that in case of an already constructed object, the object is created via assignment. However, this contradicts the original use case that led to etl::typed_storage in the first place: #1023 The goal is to omit destructors (and at the same time support classes with deleted assignment operators), so they can be optimized out at link time. This change reverts commit ac7b268 to restore the original functionality and changes the test to reflect the original use case. * Fix missing create() in non-C++11 typed_storage_ext constructor * Typo fix --------- Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: Drew Rife <[email protected]> Co-authored-by: John Wellbelove <[email protected]> * removed navis file from project * Updated version and release notes * Removed forced unsigned int cast in type_def bit-shift operators (#1178) * Removed UB in type_def bit-shift operators * Changed shift operators to allow both signed and unsigned operands for shifts This allows the library user to explicitly use unsigned values to avoid UB * Fixed constexpr errors for CPP11 * Changed is_arithmetic checks to use is_integral since valid shifts require integral operands * Removed need for CPP11 since changes are CPP03 compatible * Delete project navis files * Add force CI check on piull requests * Removed ETL_NOEXCEPT from delegate operator(), call_if(), and call_or() Removed ETL_NOEXCEPT from closureoperator(), call_if(), and call_or() * Updated version and release notes * Updated version and release notes * Remove noexcept from delegate method stubs. (#1185) In addition to removing noexcept from call_if, this is also needed to prevent an abort when cancelling a pthread that is executing a delegate. * Updated version and release notes * Re architect the extra checks * Add CHECK_EXTRA * Fix newline at end of file * The check index macro also needs to be defined to throw * Remove ETL_VERBOSE_ERRORS macros --------- Co-authored-by: Roland Reichwein <[email protected]> Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: Drew Rife <[email protected]> Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: David Ockey <[email protected]> Co-authored-by: Marco Nilsson <[email protected]>
jwellbelove
added a commit
that referenced
this pull request
Oct 11, 2025
* Regression fix: Support zero arguments emplace() in etl::optional (#1183) * Added coderabbitai configuration * Added builtin mem function tests * Modified etl::typed_storage * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Added ETL_NOEXCEPT and ETL_NOEXCEPT_IF_NO_THROW * Added etl::typed_storage_ext and swap for same * Added etl::typed_storage_ext and swap for same # Conflicts: # include/etl/alignment.h * Added release notes * Fixes to GCC -O2 errors * Changed char* parameters to value_type* parameters * Fixed compilation issues for const containers unit tests * Added automatic selection of __builtin_memxxx functions for GCC and clang * Added enhanced coderabbit configuration * Updated version and release notes * Disabled constexpr const container tests for C++11 * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Updated version and release notes * feat: removed unreachable break statements (#1169) * Updated version and release notes * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Support zero arguments emplace() in etl::optional For non-fundamental types, a recent change in etl::optional was introduced that doesn't support zero arguments emplace() anymore. This change fixes it and adds the respective test. --------- Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: Drew Rife <[email protected]> * Fix etl::typed_storage by supporting omitted destructors (#1182) * Added coderabbitai configuration * Added builtin mem function tests * Modified etl::typed_storage * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Added ETL_NOEXCEPT and ETL_NOEXCEPT_IF_NO_THROW * Added etl::typed_storage_ext and swap for same * Added etl::typed_storage_ext and swap for same # Conflicts: # include/etl/alignment.h * Added release notes * Fixes to GCC -O2 errors * Changed char* parameters to value_type* parameters * Fixed compilation issues for const containers unit tests * Added automatic selection of __builtin_memxxx functions for GCC and clang * Added enhanced coderabbit configuration * Updated version and release notes * Disabled constexpr const container tests for C++11 * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Updated version and release notes * feat: removed unreachable break statements (#1169) * Updated version and release notes * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Fix etl::typed_storage by supporting omitted destructors In a recent change to alignment.h, the etl::typed_storage was changed in a way that in case of an already constructed object, the object is created via assignment. However, this contradicts the original use case that led to etl::typed_storage in the first place: #1023 The goal is to omit destructors (and at the same time support classes with deleted assignment operators), so they can be optimized out at link time. This change reverts commit ac7b268 to restore the original functionality and changes the test to reflect the original use case. * Fix missing create() in non-C++11 typed_storage_ext constructor * Typo fix --------- Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: Drew Rife <[email protected]> Co-authored-by: John Wellbelove <[email protected]> * removed navis file from project * Updated version and release notes * Removed forced unsigned int cast in type_def bit-shift operators (#1178) * Removed UB in type_def bit-shift operators * Changed shift operators to allow both signed and unsigned operands for shifts This allows the library user to explicitly use unsigned values to avoid UB * Fixed constexpr errors for CPP11 * Changed is_arithmetic checks to use is_integral since valid shifts require integral operands * Removed need for CPP11 since changes are CPP03 compatible * Delete project navis files * Add force CI check on piull requests * Removed ETL_NOEXCEPT from delegate operator(), call_if(), and call_or() Removed ETL_NOEXCEPT from closureoperator(), call_if(), and call_or() * Updated version and release notes * Updated version and release notes * Remove noexcept from delegate method stubs. (#1185) In addition to removing noexcept from call_if, this is also needed to prevent an abort when cancelling a pthread that is executing a delegate. * Updated version and release notes * Re architect the extra checks * Add CHECK_EXTRA * Fix newline at end of file * The check index macro also needs to be defined to throw * Remove ETL_VERBOSE_ERRORS macros --------- Co-authored-by: Roland Reichwein <[email protected]> Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: Drew Rife <[email protected]> Co-authored-by: John Wellbelove <[email protected]> Co-authored-by: David Ockey <[email protected]> Co-authored-by: Marco Nilsson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using negative numbers for the number of places that a bit-shift operator will shift the value is Undefined Behavior. This PR changes all bit-shift operators for the
type_defclass to allow explicitly using unsigned values to avoid UB. This will assist compilers and static analyzer tools to ensure that UB isn't being used. But this also still allows using signed integers for those who need it.Summary by CodeRabbit
Refactor
Tests