Skip to content
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

[onert] Use getUses() copy instead of reference #13931

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

jyoungyun
Copy link
Contributor

This commit fixes the problem that occurs in loop traversal due to change of list by using copy instead of reference.

ONE-DCO-1.0-Signed-off-by: Jiyoung Yun [email protected]

@jyoungyun jyoungyun added the PR/ready for review It is ready to review. Please review it. label Sep 4, 2024
@jyoungyun jyoungyun requested review from ragmani and a team September 4, 2024 06:42
ragmani
ragmani previously approved these changes Sep 4, 2024
Copy link
Contributor

@ragmani ragmani left a comment

Choose a reason for hiding this comment

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

LGTM

@jyoungyun jyoungyun added the approval: 1 Require at least 1 approval label Sep 4, 2024
@ragmani
Copy link
Contributor

ragmani commented Sep 4, 2024

This is related to #13803.
@hseok-oh The static analyzer may grumble with the warning. Is it OK to merge this change now?

@jyoungyun jyoungyun changed the title [onert] Copy OperationIndexSets instead of using reference [onert] Use getUses() copy instead of reference Sep 4, 2024
@jyoungyun jyoungyun removed the approval: 1 Require at least 1 approval label Sep 4, 2024
ragmani
ragmani previously approved these changes Sep 4, 2024
@hseok-oh
Copy link
Contributor

hseok-oh commented Sep 4, 2024

This is related to #13803. @hseok-oh The static analyzer may grumble with the warning. Is it OK to merge this change now?

One solution is using type name, not auto.
Other solution is using copy constructor explicitly: const auto uses(object.getUses());

@hseok-oh
Copy link
Contributor

hseok-oh commented Sep 4, 2024

@jyoungyun You cannot check this issue by svace. This was warned by coverity.

@jyoungyun
Copy link
Contributor Author

jyoungyun commented Sep 4, 2024

@hseok-oh

You cannot check this issue by svace. This was warned by coverity.

Ah, thank you. I will test it using coverity target.

This commit fixes the problem that occurs in loop traversal due to
change of list by using copy instead of reference.

ONE-DCO-1.0-Signed-off-by: Jiyoung Yun <[email protected]>
@jyoungyun
Copy link
Contributor Author

@hseok-oh When I checked this PR using the coverity tool and it did not find out any issues. Thank you.

Copy link
Contributor

@hseok-oh hseok-oh left a comment

Choose a reason for hiding this comment

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

LGTM

@hseok-oh hseok-oh merged commit 5c34723 into Samsung:master Sep 5, 2024
9 checks passed
@jyoungyun jyoungyun deleted the 240904/use_copy branch September 5, 2024 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/ready for review It is ready to review. Please review it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants