Skip to content

C++: Accept regressions after github/codeql#23116 conflation fixes. - #1220

Merged
jketema merged 1 commit into
github:nextfrom
MathiasVP:update-queries-after-23116
Nov 13, 2026
Merged

jketema merged 1 commit into
github:nextfrom
MathiasVP:update-queries-after-23116

Conversation

@MathiasVP

Copy link
Copy Markdown
Contributor

Description

github/codeql#23116 fixes a conflation problem when picking which dataflow node should satisfy node.asExpr() after returning from a constructor call. This results in some regressions on Coding Standards queries on the next branch that will be fixed after a subsequent PR I'm opening soon on the CodeQL repo.

Change request type

  • Release or process automation (GitHub workflows, internal scripts)
  • Internal documentation
  • External documentation
  • Query files (.ql, .qll, .qls or unit tests)
  • External scripts (analysis report or other code shipped as part of a release)

Rules with added or modified queries

  • No rules added
  • Queries have been added for the following rules:
    • rule number here
  • Queries have been modified for the following rules:
    • rule number here

Release change checklist

A change note (development_handbook.md#change-notes) is required for any pull request which modifies:

  • The structure or layout of the release artifacts.
  • The evaluation performance (memory, execution time) of an existing query.
  • The results of an existing query in any circumstance.

If you are only adding new rule queries, a change note is not required.

Author: Is a change note required?

  • Yes
  • No

🚨🚨🚨
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.

  • Confirmed

Reviewer: Confirm that either a change note is not required or the change note is required and has been added.

  • Confirmed

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

  • Have all the relevant rule package description files been checked in?
  • Have you verified that the metadata properties of each new query is set appropriately?
  • Do all the unit tests contain both "COMPLIANT" and "NON_COMPLIANT" cases?
  • Are the alert messages properly formatted and consistent with the style guide?
  • Have you run the queries on OpenPilot and verified that the performance and results are acceptable?
    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.
  • Does the query have an appropriate level of in-query comments/documentation?
  • Have you considered/identified possible edge cases?
  • Does the query not reinvent features in the standard library?
  • Can the query be simplified further (not golfed!)

Reviewer

  • Have all the relevant rule package description files been checked in?
  • Have you verified that the metadata properties of each new query is set appropriately?
  • Do all the unit tests contain both "COMPLIANT" and "NON_COMPLIANT" cases?
  • Are the alert messages properly formatted and consistent with the style guide?
  • Have you run the queries on OpenPilot and verified that the performance and results are acceptable?
    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.
  • Does the query have an appropriate level of in-query comments/documentation?
  • Have you considered/identified possible edge cases?
  • Does the query not reinvent features in the standard library?
  • Can the query be simplified further (not golfed!)

Copilot AI dynamic review requested due to automatic review settings November 13, 2026 13:27
@jketema
jketema merged commit 3dc266e into github:next Nov 13, 2026
16 of 20 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The updated baselines leave an accepted false positive unannotated and omit the required change note documenting affected queries and accepted regressions.

2 open issues
What changed in this PR

This PR updates C++ Coding Standards test baselines to accept temporary regressions caused by the constructor-dataflow correction in github/codeql#23116. It changes expected results and test comments, not query implementations.

Changes:

  • Records new false positives in stream handling, iterator capacity, and shared-pointer ownership checks.
  • Removes expected findings for affected stream-positioning and smart-pointer cases.
  • Marks known regressions in test comments pending an upstream follow-up fix.
File Description
cpp/​common/​test/​rules/​stringnumberconversionmissingerrorcheck/​test.cpp Annotates four known false positives.
cpp/​common/​test/​rules/​stringnumberconversionmissingerrorcheck/​StringNumberConversionMissingErrorCheck.expected Adds four expected conversion-check alerts.
cpp/​common/​test/​rules/​ownedpointervaluestoredinunrelatedsmartpointer/​test.cpp Marks a smart-pointer finding as missing.
cpp/​common/​test/​rules/​ownedpointervaluestoredinunrelatedsmartpointer/​OwnedPointerValueStoredInUnrelatedSmartPointer.expected Removes affected findings and path data.
cpp/​common/​test/​rules/​iofstreammissingpositioning/​test.cpp Marks seven positioning findings as missing.
cpp/​common/​test/​rules/​iofstreammissingpositioning/​IOFstreamMissingPositioning.expected Removes seven expected positioning alerts.
cpp/​cert/​test/​rules/​FIO51-CPP/​test.cpp Annotates three file-closure false positives.
cpp/​cert/​test/​rules/​FIO51-CPP/​CloseFilesWhenTheyAreNoLongerNeeded.expected Adds alerts and shifts related locations to constructors.
cpp/​cert/​test/​rules/​CTR52-CPP/​test.cpp Annotates two iterator-capacity false positives.
cpp/​cert/​test/​rules/​CTR52-CPP/​GuaranteeGenericCppLibraryFunctionsDoNotOverflow.expected Adds two expected iterator-capacity alerts.
cpp/​autosar/​test/​rules/​A20-8-4/​test.cpp Annotates a shared-ownership false positive.
cpp/​autosar/​test/​rules/​A20-8-4/​SharedPointerUsedWithNoOwnershipSharing.expected Adds the expected shared-ownership alert.

Review effort: Balanced (dynamic)

| test.cpp:63:3:63:16 | call to terminate | The execution terminates without closing the `fstream` last accessed in $@. | test.cpp:48:16:48:19 | call to basic_fstream | call to basic_fstream |
| test.cpp:77:3:77:16 | call to terminate | The execution terminates without closing the `fstream` last accessed in $@. | test.cpp:68:22:68:28 | call to basic_fstream | call to basic_fstream |
| test.cpp:77:3:77:16 | call to terminate | The execution terminates without closing the `fstream` last accessed in $@. | test.cpp:69:22:69:29 | call to basic_fstream | call to basic_fstream |
| test.cpp:88:5:88:18 | call to terminate | The execution terminates without closing the `fstream` last accessed in $@. | test.cpp:82:21:82:27 | call to basic_fstream | call to basic_fstream |
@@ -1,8 +1,12 @@
| test.cpp:9:5:9:5 | call to operator>> | Error state not checked after call to string-to-numeric conversion function operator>>. |
| test.cpp:15:33:15:33 | call to operator>> | Error state not checked after call to string-to-numeric conversion function operator>>. |
| test.cpp:51:5:51:5 | call to operator>> | Error state not checked after call to string-to-numeric conversion function operator>>. |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants