Add -Wunsafe-buffer-usage to clang 16+ builds#804
Conversation
WalkthroughAdds Clang 16+ warning support to CMake via a new version gate Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CMake invocation
participant CMake as CMake
participant Targets as Build Targets
Note over CMake: Configure phase evaluates compiler/version
Dev->>CMake: run cmake configure
CMake->>CMake: detect compiler, set cxx_ge_16
CMake->>CMake: set is_clang_ge_16_not_windows
CMake->>CMake: define CLANG_GE_16_CXX_WARNING_FLAGS
CMake->>Targets: populate COMMON_TARGET_PROPERTIES
Note over Targets: Generator-expression applies\nCLANG_GE_16_CXX_WARNING_FLAGS when\nis_standalone AND is_clang_ge_16_not_windows
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
⏰ 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). (10)
🔇 Additional comments (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
CMakeLists.txt (3)
399-411: Missing is_gxx_ge_16 generator expression.Line 717 references
is_gxx_ge_16, but this variable is never defined. Compare with existing GCC version checks likeis_gxx_ge_14(line 411).Add the missing generator expression after line 411:
set(is_gxx_ge_12 "$<AND:${is_gxx_genex},${cxx_ge_12}>") set(is_gxx_ge_14 "$<AND:${is_gxx_genex},${cxx_ge_14}>") +set(is_gxx_ge_16 "$<AND:${is_gxx_genex},${cxx_ge_16}>")
113-118: Missing GCC_GE_16_CXX_WARNING_FLAGS definition.Line 717 references
GCC_GE_16_CXX_WARNING_FLAGS, but this variable is never defined. The pattern showsGCC_GE_14_CXX_WARNING_FLAGSat line 117, andGCC_GE_11_CXX_WARNING_FLAGSat line 113.Add the missing variable definition after line 118:
set(GCC_GE_14_CXX_WARNING_FLAGS "-Wnrvo" "-Welaborated-enum-base" "-Wdangling-reference") +set(GCC_GE_16_CXX_WARNING_FLAGS "-Wunsafe-buffer-usage")
704-717: Critical variables are undefined in CMakeLists.txt.Based on the search results, the original review is correct. Both
is_gxx_ge_16andGCC_GE_16_CXX_WARNING_FLAGSreferenced at line 717 are not defined anywhere in the file. The comprehensive regex search (CLANG_GE_16|GCC_GE_16|is_gxx_ge_16|is_clang_ge_16) returned only three matches:
- Line 89:
CLANG_GE_16_CXX_WARNING_FLAGSis defined- Line 408:
is_clang_ge_16_not_windowsis defined- Line 717: Both undefined variables are referenced
Additionally, while
CLANG_GE_16_CXX_WARNING_FLAGSandis_clang_ge_16_not_windowsare defined earlier, they are not used in the warning flags section (lines 704-717). This creates an inconsistency: Clang 16+ support is configured but not applied.Fix line 717 by either:
- Defining the missing GCC and generator expression variables earlier in the file, or
- Adding a corresponding line for Clang 16+ using the already-defined
is_clang_ge_16_not_windows
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
CMakeLists.txt(3 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). (10)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Static Code Analysis
laurynas-biveinis
force-pushed
the
clang-wunsafe-buffer-usage
branch
from
b800c6a to
2332215
Compare
| "-Wweak-template-vtables" "-Wweak-vtables" "-Wzero-as-null-pointer-constant") | ||
| set(CLANG_LT_13_CXX_WARNING_FLAGS "-Wreserved-id-macro") | ||
| set(CLANG_GE_13_CXX_WARNING_FLAGS "-Wreserved-identifier") | ||
| set(CLANG_GE_16_CXX_WARNING_FLAGS "-Wunsafe-buffer-usage") |
There was a problem hiding this comment.
🧩 Analysis chain
Previous issue resolved: CLANG_GE_16_CXX_WARNING_FLAGS is now properly applied.
The four additions correctly implement Clang 16+ warning support, and the flag is now applied in COMMON_TARGET_PROPERTIES at line 713, addressing the prior feedback. The implementation follows the established pattern for version-gated warning flags.
Verify that -Wunsafe-buffer-usage is a valid Clang 16+ flag:
Also applies to: 404-404, 408-408, 713-713
🌐 Web query:
Is -Wunsafe-buffer-usage supported in Clang 16?
💡 Result:
Short answer: No. Clang 16 does not support -Wunsafe-buffer-usage. Clang's -Wunsafe-buffer-usage (and the related unsafe_buffer_usage attribute/features) were added later (see Clang diagnostics and related commits/PRs from 2023–2025). [1][2][3]
Sources:
- Clang 16 release notes (no -Wunsafe-buffer-usage listed). [1]
- Clang diagnostics/docs showing -Wunsafe-buffer-usage (later Clang docs / DiagnosticsReference). [2]
- Clang commits / PRs adding unsafe_buffer_usage / -Wunsafe-buffer-usage enhancements (2023–2025). [3]
Remove -Wunsafe-buffer-usage flag—it's not supported in Clang 16.
Clang 16 does not support -Wunsafe-buffer-usage. The flag was added in later versions. Remove the variable definition and all references to it: lines 89, 404, 408, and 713 in CMakeLists.txt. If Clang 16+ support for unsafe buffer warnings is needed, verify the actual flag name and minimum Clang version that supports it.
🤖 Prompt for AI Agents
CMakeLists.txt lines ~89,404,408,713: the variable set at line 89 defining
CLANG_GE_16_CXX_WARNING_FLAGS with "-Wunsafe-buffer-usage" and its uses at lines
404, 408 and 713 must be removed because Clang 16 doesn't support that flag;
delete the set(...) line and remove or replace the three references where the
variable is appended to compiler flags (lines 404, 408, 713), and if you need
the warning on newer Clang only, wrap the flag in a conditional that checks the
Clang version before adding it (or document the minimum Clang version and the
correct flag name) so no unsupported flag is added for Clang 16.
laurynas-biveinis
force-pushed
the
clang-wunsafe-buffer-usage
branch
from
2332215 to
dc17f7a
Compare
|
1 participant
Summary by CodeRabbit