Add MISRA C++:2023 warning features for gcc and clang - #43
nradakovic wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Both reviewed files have unresolved moderate findings requiring documentation or validation updates.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds compiler-specific Bazel features for MISRA C++:2023 warning diagnostics, scoped to C++ compilation actions.
Changes:
- Adds GCC and Clang warning mappings.
- Exposes
misra_cpp_2023_warningsfeature targets. - Omits clang-tidy integration for separate handling.
File summaries
| File | Summary |
|---|---|
warnings/gcc/features/misra_cpp_2023/BUILD |
Defines GCC warning arguments and feature. Moderate documentation issue remains (3 votes), including GEP traceability and partial-enforcement limitations. |
warnings/clang/features/misra_cpp_2023/BUILD |
Defines Clang warning arguments and feature. Moderate validation issue remains (3 votes); representative compile and C compatibility tests are needed. |
Review details
Suppressed comments (1)
warnings/gcc/features/misra_cpp_2023/BUILD:55
- This adds a public GCC feature, but the repository's existing warning test infrastructure never registers or enables it. Without a representative GCC positive/negative compile test (and a C target proving these C++-only args do not affect C actions), an invalid flag or incorrect feature/action wiring can pass review unnoticed. Add the feature to a dedicated test toolchain/config and exercise the expected diagnostics in CI.
cc_feature(
name = "misra_cpp_2023",
args = ["@score_cpp_policies//warnings/gcc/features/misra_cpp_2023:args"],
feature_name = "misra_cpp_2023_warnings",
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Introduce cc_feature targets (misra_cpp_2023_warnings) with the compiler warnings listed in the S-CORE MISRA C++:2023 Guideline Enforcement Plan. Warnings are scoped to C++ compile actions only, since several flags are invalid for C. clang-tidy checks are intentionally left out; their integration will be handled separately via the clang_tidy policy module. resolves #42
f2bc2a4 to
33b809c
Compare
| | `-Wuninitialized` | Warn about variables used before being initialized on some code path. | | ||
| | `-Wduplicate-enum` | Warn about two enumerators in the same enum sharing the same value where it looks unintentional. | | ||
| | `-Winconsistent-missing-destructor-override` | Warn when a class overrides a base class's virtual destructor without marking its own `override`. | | ||
| | `-Winconsistent-missing-override` | Warn when a virtual function overrides a base-class member but isn't itself marked `override`. | |
There was a problem hiding this comment.
Out of curiosity was -Wsuggest-destructor-override ruled out?
It complements -Winconsistent-missing-override.
class Base {
public:
// Added {} here so the function has an implementation
virtual void doSomething() {};
virtual ~Base() {};
};
class Derived : public Base {
public:
void doSomething() override { }; // Uses override here!
~Derived() {}; // Missing override here!
};
int main() {
Derived d;
}complied with just -std=c++17 -Winconsistent-missing-override
does not produce the warning.
<source>:11:5: warning: '~Derived' overrides a destructor but is not marked 'override' [-Wsuggest-destructor-override]
11 | ~Derived() {}; // Missing override here!Can be tested here -> https://godbolt.org/z/zj5bzqshz
There was a problem hiding this comment.
It helps with "Rule 13.3.1 User-declared member functions shall use the virtual, override and final specifiers appropriately."
Introduce cc_feature targets (misra_cpp_2023_warnings) with the compiler warnings listed in the S-CORE MISRA C++:2023 Guideline Enforcement Plan. Warnings are scoped to C++ compile actions only, since several flags are invalid for C. clang-tidy checks are intentionally left out; their integration will be handled separately via the clang_tidy policy module.
resolves #42