Skip to content

C++: add predicate to distinguish designator-based initializations #19329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 17, 2025

Conversation

IdrissRio
Copy link
Contributor

Introduced isDesignatorInit() predicates to distinguish between designator-based and positional initializations for both struct\union fields and array elements.

@IdrissRio IdrissRio force-pushed the idrissrio/designated-initializer branch from a3371dc to caa2fff Compare April 16, 2025 14:32
@IdrissRio IdrissRio added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Apr 16, 2025
@IdrissRio IdrissRio force-pushed the idrissrio/designated-initializer branch from caa2fff to cc49cfc Compare April 16, 2025 15:07
@IdrissRio IdrissRio force-pushed the idrissrio/designated-initializer branch from cc49cfc to 15fe2fb Compare April 16, 2025 18:45
@IdrissRio IdrissRio marked this pull request as ready for review April 16, 2025 20:39
@Copilot Copilot AI review requested due to automatic review settings April 16, 2025 20:39
@IdrissRio IdrissRio requested a review from a team as a code owner April 16, 2025 20:39
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces predicates to distinguish designator-based initializations from positional ones for struct/union fields and array elements in C++.

  • Added isDesignatorInit() predicates
  • Updated change notes to document the new feature
Files not reviewed (8)
  • cpp/downgrades/2e2d805ef93d060b813403cb9b51dc72455a4c68/aggregate_array_init.ql: Language not supported
  • cpp/downgrades/2e2d805ef93d060b813403cb9b51dc72455a4c68/aggregate_field_init.ql: Language not supported
  • cpp/downgrades/2e2d805ef93d060b813403cb9b51dc72455a4c68/upgrade.properties: Language not supported
  • cpp/ql/lib/semmle/code/cpp/exprs/Literal.qll: Language not supported
  • cpp/ql/lib/semmlecode.cpp.dbscheme: Language not supported
  • cpp/ql/lib/upgrades/0f0a390468a5eb43d1dc72937c028070b106bf53/aggregate_array_init.ql: Language not supported
  • cpp/ql/lib/upgrades/0f0a390468a5eb43d1dc72937c028070b106bf53/aggregate_field_init.ql: Language not supported
  • cpp/ql/lib/upgrades/0f0a390468a5eb43d1dc72937c028070b106bf53/upgrade.properties: Language not supported

jketema
jketema previously approved these changes Apr 17, 2025
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

Some minor comments on the QLDoc, otherwise LGTM

Comment on lines 223 to 225
* This can be used to distinguish explicitly designated initializations from
* implicit positional ones.
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* This can be used to distinguish explicitly designated initializations from
* implicit positional ones.
*

Comment on lines 231 to 232
* - `.x = 1` is a designator init, therefore `isDesignatorInit(x, 0)` holds.
* - `2` is a positional init for `.y`, therefore `isDesignatorInit(y, 1)` does **not** hold.
Copy link
Contributor

@jketema jketema Apr 17, 2025

Choose a reason for hiding this comment

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

Suggested change
* - `.x = 1` is a designator init, therefore `isDesignatorInit(x, 0)` holds.
* - `2` is a positional init for `.y`, therefore `isDesignatorInit(y, 1)` does **not** hold.
* - `.x = 1` uses a designator, therefore `hasDesignator(x, 0)` holds.
* - `2` is positional for `.y`, therefore `hasDesignator(y, 1)` does not hold.


/**
* Holds if the `position`-th initialization of `field` in this aggregate initializer
* uses a designator (e.g., `.x =`, `[42] =`) rather than a positional initializer.
Copy link
Contributor

@jketema jketema Apr 17, 2025

Choose a reason for hiding this comment

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

Suggested change
* uses a designator (e.g., `.x =`, `[42] =`) rather than a positional initializer.
* uses a designated (e.g., `.x = ...`) rather than a positional initializer.

Comment on lines 340 to 341
* - `[0] = 1` is a designator init, therefore `isDesignatorInit(0, 0)` holds.
* - `2` is a positional init for `x[1]`, therefore `isDesignatorInit(1, 1)` does **not** hold.
Copy link
Contributor

@jketema jketema Apr 17, 2025

Choose a reason for hiding this comment

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

Suggested change
* - `[0] = 1` is a designator init, therefore `isDesignatorInit(0, 0)` holds.
* - `2` is a positional init for `x[1]`, therefore `isDesignatorInit(1, 1)` does **not** hold.
* - `[0] = 1` uses a designator, therefore `hasDesignator(0, 0)` holds.
* - `2` is positional for `x[1]`, therefore `hasDesignator(1, 1)` does not hold.

---
category: feature
---
* Introduced `isDesignatorInit()` predicates to distinguish between designator-based and positional initializations for both struct/union fields and array elements.
Copy link
Contributor

@jketema jketema Apr 17, 2025

Choose a reason for hiding this comment

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

Suggested change
* Introduced `isDesignatorInit()` predicates to distinguish between designator-based and positional initializations for both struct/union fields and array elements.
* Introduced `hasDesignator()` predicates to distinguish between designated and positional initializations for both struct/union fields and array elements.

* - `.x = 1` is a designator init, therefore `isDesignatorInit(x, 0)` holds.
* - `2` is a positional init for `.y`, therefore `isDesignatorInit(y, 1)` does **not** hold.
*/
predicate isDesignatorInit(Field field, int position) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
predicate isDesignatorInit(Field field, int position) {
predicate hasDesignator(Field field, int position) {

Comment on lines 332 to 334
* Holds if the `position`-th initialization of the array element at `elementIndex`
* in this aggregate initializer uses a designator (e.g., `[0] = ...`) rather than
* an implicit positional initializer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Holds if the `position`-th initialization of the array element at `elementIndex`
* in this aggregate initializer uses a designator (e.g., `[0] = ...`) rather than
* an implicit positional initializer.
* Holds if the `position`-th initialization of the array element at `elementIndex`
* in this aggregate initializer uses a designated (e.g., `[0] = ...`) rather than
* an positional initializer.

* - `[0] = 1` is a designator init, therefore `isDesignatorInit(0, 0)` holds.
* - `2` is a positional init for `x[1]`, therefore `isDesignatorInit(1, 1)` does **not** hold.
*/
predicate isDesignatorInit(int elementIndex, int position) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
predicate isDesignatorInit(int elementIndex, int position) {
predicate hasDesignator(int elementIndex, int position) {

@jketema jketema merged commit 53bd236 into main Apr 17, 2025
13 of 16 checks passed
@jketema jketema deleted the idrissrio/designated-initializer branch April 17, 2025 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants