-
Notifications
You must be signed in to change notification settings - Fork 0
wider _GLIBCXX_DEBUG_BACKTRACE support #6
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
Conversation
Ugly scattering of try_compile tests for the different ways backtrace support can be provided by libstdc++
WalkthroughThis pull request updates several CMake configuration files. In the main CMakeLists.txt and test/CMakeLists.txt, the copyright year is extended to “2024-2025,” and Git tags for various dependencies are updated. The test CMakeLists.txt also refactors the compiler’s debug configuration by replacing direct flag definitions with a dedicated glibcxx_debug_backtrace.cmake file, which introduces a series of compile tests to detect support for the GLIBCXX_DEBUG_BACKTRACE feature. Changes
Sequence Diagram(s)sequenceDiagram
participant CMake
participant DebugCheck
participant Compiler
CMake->>DebugCheck: Invoke glibcxx_debug_backtrace.cmake
DebugCheck->>Compiler: Attempt basic compilation test
Compiler-->>DebugCheck: Return compilation result
alt Basic test successful
DebugCheck->>CMake: Set GLIBCXX_DEBUG_BACKTRACE_SUPPORTED = true
else Basic test failed
DebugCheck->>Compiler: Attempt compile with C++23 & stdc++exp
Compiler-->>DebugCheck: Return compilation result
alt Alternate test successful
DebugCheck->>CMake: Configure library settings and set support flag
else All attempts failed
DebugCheck->>CMake: Issue warning about unsupported GLIBCXX_DEBUG_BACKTRACE
end
end
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
CMakeLists.txt(3 hunks)test/CMakeLists.txt(3 hunks)test/glibcxx_debug_backtrace.cmake(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (windows-latest, Release, cl)
- GitHub Check: build (windows-latest, Debug, cl)
🔇 Additional comments (4)
CMakeLists.txt (2)
25-25: Verify Git tag for decodeless_allocator.Please ensure that this commit hash points to a stable version of decodeless_allocator.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify Git tag for decodeless_allocator. gh api repos/decodeless/allocator/commits/9b45b23ea556ae5ec42af4ce60b13bd3a7db8a8f \ --jq '.commit.message'Length of output: 154
Git tag for decodeless_allocator verified.
The commit hash "9b45b23ea556ae5ec42af4ce60b13bd3a7db8a8f" references the commit with the message "fix zero initial size for memory resource parent," indicating a deliberate bug fix. This confirms that the provided Git tag points to a valid and stable commit in decodeless_allocator.
46-46: Verify Git tag for decodeless_mappedfile.Please ensure that this commit hash points to a stable version of decodeless_mappedfile.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify Git tag for decodeless_mappedfile. gh api repos/decodeless/mappedfile/commits/a6e59f8b193298a16534beab694e30ae76d7f457 \ --jq '.commit.message'Length of output: 145
Verification Successful: Commit Tag Points to a Stable Version
- The commit hash a6e59f8b193298a16534beab694e30ae76d7f457 returns the commit message “wider _GLIBCXX_DEBUG_BACKTRACE support.”
- This message indicates an enhancement rather than instability, which implies that the tag refers to a stable version.
test/CMakeLists.txt (2)
53-64: LGTM! Well-structured feature support.The implementation of _GLIBCXX_DEBUG_BACKTRACE support is well-structured:
- Clear comment explaining the purpose
- Proper inclusion of the detection script
- Conditional compilation and linking based on feature support
- Appropriate handling of C++ feature requirements
27-27: Verify Git tag for decodeless_offset_ptr.Please ensure that this commit hash points to a stable version of decodeless_offset_ptr.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify Git tag for decodeless_offset_ptr. gh api repos/decodeless/offset_ptr/commits/38ceefc6ce63fb4667cd207424b1277c3eed5f8d \ --jq '.commit.message'Length of output: 126
Git tag for decodeless_offset_ptr verified.
The commit hash 38ceefc6ce63fb4667cd207424b1277c3eed5f8d returns the commit message “gdb pretty printers”, confirming that it points to a specific, valid commit. Assuming that this commit implements the intended stable feature for decodeless_offset_ptr, the tag appears appropriate for usage.
| @@ -0,0 +1,73 @@ | |||
| # Copyright (c) 2025 Pyarelal Knowles, MIT License | |||
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.
🧹 Nitpick (assertive)
Update copyright year.
The copyright year "2025" should be "2024-2025" to match other files.
-# Copyright (c) 2025 Pyarelal Knowles, MIT License
+# Copyright (c) 2024-2025 Pyarelal Knowles, MIT License📝 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.
| # Copyright (c) 2025 Pyarelal Knowles, MIT License | |
| # Copyright (c) 2024-2025 Pyarelal Knowles, MIT License |
| COMPILE_DEFINITIONS -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_BACKTRACE CXX_STANDARD | ||
| 20 CXX_STANDARD_REQUIRED true | ||
| LINK_LIBRARIES stdc++exp) |
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.
🧹 Nitpick (assertive)
Fix line continuation indentation.
The line continuation indentation is inconsistent with other similar blocks in the file.
- COMPILE_DEFINITIONS -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_BACKTRACE CXX_STANDARD
- 20 CXX_STANDARD_REQUIRED true
- LINK_LIBRARIES stdc++exp)
+ COMPILE_DEFINITIONS -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_BACKTRACE
+ CXX_STANDARD 20
+ CXX_STANDARD_REQUIRED true
+ LINK_LIBRARIES stdc++exp)📝 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.
| COMPILE_DEFINITIONS -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_BACKTRACE CXX_STANDARD | |
| 20 CXX_STANDARD_REQUIRED true | |
| LINK_LIBRARIES stdc++exp) | |
| COMPILE_DEFINITIONS -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_BACKTRACE | |
| CXX_STANDARD 20 | |
| CXX_STANDARD_REQUIRED true | |
| LINK_LIBRARIES stdc++exp) |
| message( | ||
| WARNING "No working try_compile configs with _GLIBCXX_DEBUG_BACKTRACE found" | ||
| ) |
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.
🧹 Nitpick (assertive)
Enhance warning message.
The warning message could be more informative by suggesting potential solutions.
- WARNING "No working try_compile configs with _GLIBCXX_DEBUG_BACKTRACE found"
+ WARNING "No working try_compile configs with _GLIBCXX_DEBUG_BACKTRACE found. "
+ "Try installing libstdc++_libbacktrace or upgrading to a newer compiler."| stdc++_libbacktrace_test.cpp | ||
| "#include <vector>\nint main() { std::vector<int> v{0}; return v[0]; }" | ||
| COMPILE_DEFINITIONS -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_BACKTRACE) |
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.
🧹 Nitpick (assertive)
Consider adding more test cases.
The test program only checks vector access. Consider adding more test cases that exercise different aspects of the debug functionality.
- "#include <vector>\nint main() { std::vector<int> v{0}; return v[0]; }"
+ "#include <vector>\n#include <string>\nint main() {\n"
+ " std::vector<int> v{0};\n"
+ " std::string s;\n"
+ " s.at(0); // Should trigger debug assertion\n"
+ " return v[0];\n"
+ "}"
Ugly scattering of try_compile tests for the different ways backtrace support can be provided by libstdc++
Summary by CodeRabbit