cpp: model standard and BDE optional contents - #22582
Open
kumarak wants to merge 1 commit into
Open
kumarak wants to merge 1 commit into
kumarak wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Accessor write-back depth and standard scalar conversion coverage are incomplete.
Get a fresh assessment by requesting another Copilot review.
Pull request overviewAdds C++ dataflow models for values contained in standard and BDE optional types.
Changes:
- Models optional construction, assignment, emplacement, and accessors.
- Supports aggregate and pointer-content propagation.
- Adds regression tests and a change note.
| File | Description |
|---|---|
| cpp/ql/lib/semmle/code/cpp/models/Models.qll | Registers optional models. |
| cpp/ql/lib/semmle/code/cpp/models/implementations/Optional.qll | Implements construction and mutation summaries. |
| cpp/ql/lib/ext/optional.model.yml | Defines accessor flow summaries. |
| cpp/ql/lib/change-notes/2026-09-15-optional.md | Documents the new modeling. |
| cpp/ql/test/library-tests/dataflow/optional/optional.h | Provides reduced optional APIs. |
| cpp/ql/test/library-tests/dataflow/optional/test.cpp | Tests core optional operations. |
| cpp/ql/test/library-tests/dataflow/optional/emplacement.cpp | Tests emplacement body analysis. |
| cpp/ql/test/library-tests/dataflow/optional/conversion.cpp | Tests user-defined conversions. |
| cpp/ql/test/library-tests/dataflow/optional/flow.ql | Configures optional flow tests. |
| cpp/ql/test/library-tests/dataflow/optional/flow.expected | Stores generated expectations. |
| cpp/ql/test/library-tests/dataflow/optional-std-backed/test.cpp | Tests std-backed BDE optionals. |
| cpp/ql/test/library-tests/dataflow/optional-std-backed/flow.ql | Configures std-backed tests. |
| cpp/ql/test/library-tests/dataflow/optional-std-backed/flow.expected | Stores generated expectations. |
Suppressed comments (8)
cpp/ql/lib/ext/optional.model.yml:12
- The write-back alias for operator* is capped at one pointee even though the read model supports all @ depths. Writes through an optional containing a multiply-indirect pointer therefore disappear before a later dereference.
- ["std", "optional", False, "operator*", "", "", "ReturnValue[*]", "Argument[-1].Element[]", "value", "manual"]
- ["std", "optional", False, "operator*", "", "", "ReturnValue[**]", "Argument[-1].Element[*]", "value", "manual"]
cpp/ql/lib/ext/optional.model.yml:15
- The reverse operator-> model only covers Element[] and Element[*], unlike its wildcard forward alias. This loses writes through deeper pointer payloads returned by operator->.
- ["std", "optional", False, "operator->", "", "", "ReturnValue[*]", "Argument[-1].Element[]", "value", "manual"]
- ["std", "optional", False, "operator->", "", "", "ReturnValue[**]", "Argument[-1].Element[*]", "value", "manual"]
cpp/ql/lib/ext/optional.model.yml:18
- The BSL value reverse alias is limited to one pointee while the forward alias supports all modeled depths, so writes through multiply-indirect pointer payloads are not retained.
- ["bsl", "optional", False, "value", "", "", "ReturnValue[*]", "Argument[-1].Element[]", "value", "manual"]
- ["bsl", "optional", False, "value", "", "", "ReturnValue[**]", "Argument[-1].Element[*]", "value", "manual"]
cpp/ql/lib/ext/optional.model.yml:21
- The BSL operator* write-back model stops at Element[*], producing false negatives for writes through deeper pointer payloads despite the wildcard read model.
- ["bsl", "optional", False, "operator*", "", "", "ReturnValue[*]", "Argument[-1].Element[]", "value", "manual"]
- ["bsl", "optional", False, "operator*", "", "", "ReturnValue[**]", "Argument[-1].Element[*]", "value", "manual"]
cpp/ql/lib/ext/optional.model.yml:24
- The BSL operator-> reverse alias omits depths beyond one pointee, so deeper writes cannot flow back into the optional's synthetic element contents.
- ["bsl", "optional", False, "operator->", "", "", "ReturnValue[*]", "Argument[-1].Element[]", "value", "manual"]
- ["bsl", "optional", False, "operator->", "", "", "ReturnValue[**]", "Argument[-1].Element[*]", "value", "manual"]
cpp/ql/lib/ext/optional.model.yml:27
- Optional_Base::value has the same asymmetric depth limit: reads expand through all supported element depths, but writes are retained for only the first two. Use the wildcard reverse alias so inherited BDE accessors preserve deeper writes.
- ["BloombergLP::bslstl", "Optional_Base", False, "value", "", "", "ReturnValue[*]", "Argument[-1].Element[]", "value", "manual"]
- ["BloombergLP::bslstl", "Optional_Base", False, "value", "", "", "ReturnValue[**]", "Argument[-1].Element[*]", "value", "manual"]
cpp/ql/lib/ext/optional.model.yml:30
- Optional_Base::operator* drops write-back flow beyond one pointee, although its forward alias supports the full @ range. This leaves deeper BDE pointer payload writes unmodeled.
- ["BloombergLP::bslstl", "Optional_Base", False, "operator*", "", "", "ReturnValue[*]", "Argument[-1].Element[]", "value", "manual"]
- ["BloombergLP::bslstl", "Optional_Base", False, "operator*", "", "", "ReturnValue[**]", "Argument[-1].Element[*]", "value", "manual"]
cpp/ql/lib/ext/optional.model.yml:33
- Optional_Base::operator-> also restricts reverse flow to one pointee. A wildcard reverse alias is needed to match the forward model and preserve writes through deeper pointer payloads.
- ["BloombergLP::bslstl", "Optional_Base", False, "operator->", "", "", "ReturnValue[*]", "Argument[-1].Element[]", "value", "manual"]
- ["BloombergLP::bslstl", "Optional_Base", False, "operator->", "", "", "ReturnValue[**]", "Argument[-1].Element[*]", "value", "manual"]
- Files reviewed: 11/13 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
Comment thread
cpp/ql/lib/ext/optional.model.yml
Comment on lines
+8
to
+9
| - ["std", "optional", False, "value", "", "", "ReturnValue[*]", "Argument[-1].Element[]", "value", "manual"] | ||
| - ["std", "optional", False, "value", "", "", "ReturnValue[**]", "Argument[-1].Element[*]", "value", "manual"] |
Comment on lines
+48
to
+52
| ( | ||
| sourceType = targetType | ||
| or | ||
| sourceType instanceof ArithmeticType and targetType instanceof ArithmeticType | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change models contained-value flow through std::optional and bsl::optional, including construction, copy/move assignment, value, operator*, operator->, and single-argument scalar emplacement.
The models preserve aggregate fields and write through the returned references, including BDE accessors on Optional_Base and configurations that inherit from std::optional. User-defined conversions and emplacement constructors remain available for body analysis.
Tests cover scalar, pointer, and aggregate payloads; const access; reference writes; copy/move operations; and constructors that use or ignore their inputs.
The engagement state and the clearing of historical taint after reset or replacement are modeled conservatively. Allocator-tag construction, legacy move emulation, value_or, and monadic operations are out of scope.