Initial implementation of rule 4-1-1, must be standard C++17#1106
Initial implementation of rule 4-1-1, must be standard C++17#1106MichaelRFairhurst wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds the initial CodeQL implementation for MISRA C++:2023 RULE-4-1-1 (“must be standard C++17”) by modeling several common compiler-language extensions and wiring the rule into the toolchain package and test suite.
Changes:
- Introduces a new rule query (
CompilerLanguageExtensionsUsed) and a shared library model of C++ compiler extensions. - Adds a dedicated unit test directory with a
.qlrefand.expectedresults for RULE-4-1-1. - Wires the rule into
Toolchain2packaging/metadata and updates standard-library test stubs needed by the test.
Show a summary per file
| File | Description |
|---|---|
| rule_packages/cpp/Toolchain2.json | Adds MISRA-C++-2023 RULE-4-1-1 metadata and query descriptor in Toolchain2 package. |
| cpp/misra/src/rules/RULE-4-1-1/CompilerLanguageExtensionsUsed.ql | New rule query selecting detected compiler extensions and reporting messages. |
| cpp/common/src/codingstandards/cpp/Extensions.qll | Adds the core modeling of C++ compiler extensions and associated alert messages. |
| cpp/misra/test/rules/RULE-4-1-1/test.cpp | New test cases covering attributes, builtins, statement expressions, pragmas, etc. |
| cpp/misra/test/rules/RULE-4-1-1/CompilerLanguageExtensionsUsed.qlref | Test reference to the production query. |
| cpp/misra/test/rules/RULE-4-1-1/CompilerLanguageExtensionsUsed.expected | Expected results for the test query execution. |
| cpp/common/test/includes/standard-library/type_traits.h | Extends the type-traits stub to support test compilation. |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/Toolchain2.qll | Autogenerated Toolchain2 query metadata wrapper for exclusions. |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll | Registers Toolchain2 metadata with the exclusions framework. |
Copilot's findings
Comments suppressed due to low confidence (2)
cpp/common/src/codingstandards/cpp/Extensions.qll:126
- Some message strings exceed the repo's 100-character line limit for CodeQL source/test files (for example, this line). Please wrap these strings across multiple lines to keep each source line ≤ 100 characters.
override string getMessage() {
result =
"Extended floating-point types are a compiler extension and are not portable to other compilers."
}
cpp/common/src/codingstandards/cpp/Extensions.qll:203
CPPPreprocessorDirectiveExtensionis a new public class (not markedprivate) but it does not have a QLDoc comment. Add QLDoc (or mark itprivate/ document it as INTERNAL) to satisfy the project's documentation requirements for public declarations.
class CPPPreprocessorDirectiveExtension extends CPPCompilerExtension, PreprocessorDirective {
string kind;
CPPPreprocessorDirectiveExtension() {
this instanceof PreprocessorPragma and kind = "#pragma " + getHead()
or
- Files reviewed: 9/9 changed files
- Comments generated: 8
| from CPPCompilerExtension e | ||
| where not isExcluded(e, Toolchain2Package::compilerLanguageExtensionsUsedQuery()) | ||
| select MacroUnwrapper<CPPCompilerExtension>::unwrapElement(e), e.getMessage() |
There was a problem hiding this comment.
The query is currently reporting a result whose primary location is inside the test standard-library stub (includes/standard-library/stdarg.h), which leads to machine-specific absolute file:///... paths and makes the test not correspond to a NON_COMPLIANT marker in test.cpp. Consider adjusting the reporting element (or adding a filter) so results are reported in user/test source files and not in includes/standard-library stubs (for example, mirror the standard-library exclusion pattern used elsewhere in cpp/common/src/codingstandards/cpp/Macro.qll).
| from CPPCompilerExtension e | |
| where not isExcluded(e, Toolchain2Package::compilerLanguageExtensionsUsedQuery()) | |
| select MacroUnwrapper<CPPCompilerExtension>::unwrapElement(e), e.getMessage() | |
| from CPPCompilerExtension e, Element report | |
| where | |
| not isExcluded(e, Toolchain2Package::compilerLanguageExtensionsUsedQuery()) and | |
| ( | |
| report = MacroUnwrapper<CPPCompilerExtension>::unwrapElement(e) and | |
| not isExcluded(report, Toolchain2Package::compilerLanguageExtensionsUsedQuery()) | |
| or | |
| report = e and | |
| isExcluded( | |
| MacroUnwrapper<CPPCompilerExtension>::unwrapElement(e), | |
| Toolchain2Package::compilerLanguageExtensionsUsedQuery() | |
| ) | |
| ) | |
| select report, e.getMessage() |
| #ifdef __cplusplus // COMPLIANT | ||
| #endif | ||
|
|
||
| #pragma once |
There was a problem hiding this comment.
#pragma once produces a result in the .expected file, but this line in test.cpp is not marked // NON_COMPLIANT. Update the test annotation (or adjust the expected results/query) so every expected alert corresponds to a NON_COMPLIANT case in the test source.
| #pragma once | |
| #pragma once // NON_COMPLIANT |
| @@ -0,0 +1,25 @@ | |||
| | file:///Users/michaelrfairhurst/projects/codeql-coding-standards/cpp/common/test/includes/standard-library/stdarg.h:5:1:5:47 | #define va_start(v,l) __builtin_va_start(v, l) | Use of built-in operation '__builtin_va_start' is a compiler extension. | | |||
There was a problem hiding this comment.
This expected result refers to a machine-local absolute file:///... path under cpp/common/test/includes/standard-library/.... Expected files should not contain developer-specific absolute paths, and results should ideally be reported against the test source (test.cpp) rather than the standard-library stubs; update the query/reporting element so this row becomes stable (or remove it if the rule should not flag standard va_start).
| | file:///Users/michaelrfairhurst/projects/codeql-coding-standards/cpp/common/test/includes/standard-library/stdarg.h:5:1:5:47 | #define va_start(v,l) __builtin_va_start(v, l) | Use of built-in operation '__builtin_va_start' is a compiler extension. | |
| #pragma GCC diagnostic push // NON_COMPLIANT | ||
| #warning "This is a warning" // NON_COMPLIANT | ||
| // clang-format off | ||
| # warning "preceeding spaces is common" // NON_COMPLIANT |
There was a problem hiding this comment.
Typo in the warning string: "preceeding" should be "preceding".
| # warning "preceeding spaces is common" // NON_COMPLIANT | |
| # warning "preceding spaces is common" // NON_COMPLIANT |
| override string getMessage() { | ||
| result = | ||
| "Ternaries with omitted middle operands are a compiler extension and are not portable to other compilers." | ||
| } |
There was a problem hiding this comment.
Some message strings exceed the repo's 100-character line limit for CodeQL source/test files (for example, this line). Please wrap these strings across multiple lines to keep each source line ≤ 100 characters.
This issue also appears on line 123 of the same file.
| class CppNamespacedStdAttributeExtension extends CPPCompilerExtension, StdAttribute { | ||
| CppNamespacedStdAttributeExtension() { exists(this.getNamespace()) and not getNamespace() = "" } | ||
|
|
||
| override string getMessage() { | ||
| result = | ||
| "Use of attribute '" + getName() + "' in namespace '" + getNamespace() + | ||
| "' is a compiler extension and is not portable to other compilers." | ||
| } | ||
| } | ||
|
|
||
| class CppUnrecognizedAttributeExtension extends CPPCompilerExtension, StdAttribute { | ||
| CppUnrecognizedAttributeExtension() { |
There was a problem hiding this comment.
CppNamespacedStdAttributeExtension / CppUnrecognizedAttributeExtension use a different Cpp... prefix than the rest of the newly introduced CPP... extension classes in this module. Align the naming (for example, consistently use CPP...) so the API is easier to discover and search.
| class CppUnrecognizedAttributeExtension extends CPPCompilerExtension, StdAttribute { | ||
| CppUnrecognizedAttributeExtension() { | ||
| not this instanceof CppNamespacedStdAttributeExtension and | ||
| not getName() in [ | ||
| "maybe_unused", "nodiscard", "noreturn", "deprecated", "carries_dependency", "fallthrough" | ||
| ] | ||
| } |
There was a problem hiding this comment.
These newly introduced classes are public (not marked private) but are missing QLDoc comments. The project guidelines require public declarations to be documented; add /** ... */ docs (or mark them private if they are intended for internal use only).
This issue also appears on line 198 of the same file.
| template <typename T> bool is_abstract_v = is_abstract<T>::value; | ||
|
|
||
| template <typename T, typename U> struct is_same { | ||
| const static bool value = false; | ||
| constexpr operator bool() { return value; } | ||
| }; | ||
|
|
||
| template <typename T, typename U> bool is_same_v = is_same<T, U>::value; |
There was a problem hiding this comment.
The new _v variable templates are defined as non-constexpr non-inline variables, but this same header uses inline constexpr for other _v templates (for example is_literal_type_v). Update is_abstract_v/is_same_v to follow the same pattern to avoid ODR/linkage issues and keep the stub consistent.
| template <typename T> bool is_abstract_v = is_abstract<T>::value; | |
| template <typename T, typename U> struct is_same { | |
| const static bool value = false; | |
| constexpr operator bool() { return value; } | |
| }; | |
| template <typename T, typename U> bool is_same_v = is_same<T, U>::value; | |
| template <typename T> | |
| inline constexpr bool is_abstract_v = is_abstract<T>::value; | |
| template <typename T, typename U> struct is_same { | |
| const static bool value = false; | |
| constexpr operator bool() { return value; } | |
| }; | |
| template <typename T, typename U> | |
| inline constexpr bool is_same_v = is_same<T, U>::value; |
Description
please enter the description of your change here
Change request type
.ql,.qll,.qlsor unit tests)Rules with added or modified queries
RULE-4-1-1Release change checklist
A change note (development_handbook.md#change-notes) is required for any pull request which modifies:
If you are only adding new rule queries, a change note is not required.
Author: Is a change note required?
🚨🚨🚨
Reviewer: Confirm that format of shared queries (not the .qll file, the
.ql file that imports it) is valid by running them within VS Code.
Reviewer: Confirm that either a change note is not required or the change note is required and has been added.
Query development review checklist
For PRs that add new queries or modify existing queries, the following checklist should be completed by both the author and reviewer:
Author
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
Reviewer
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.