Skip to content

C++: Fix FP in cpp/comparison-of-identical-expressions #7395

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 4 commits into from
Dec 15, 2021

Conversation

MathiasVP
Copy link
Contributor

Fixes #7385.

Initially, I wanted to add this case to Expr.isUnevaluated since that would make this fix apply more broadly. But I guess, Expr.isUnevaluated shouldn't hold for things that are evaluated at compile-time only. So for now I've just special-cased this in the query containing the actual FP report.

@MathiasVP MathiasVP added the C++ label Dec 14, 2021
@MathiasVP MathiasVP requested a review from a team as a code owner December 14, 2021 16:41
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Dec 14, 2021
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

LGTM.

I'd like to see a real-world or DCA test of some sort to confirm this doesn't affect results more widely than expected.

…flowCheck/templates.cpp

Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
@MathiasVP
Copy link
Contributor Author

LGTM.

I'd like to see a real-world or DCA test of some sort to confirm this doesn't affect results more widely than expected.

Good point. I'll do an LGTM-diff query on our usual projects since I don't think the query is actually run as part of security-extended.

@MathiasVP
Copy link
Contributor Author

No result changes on ~130 projects: https://lgtm.com/query/2133265528072233558/

@geoffw0
Copy link
Contributor

geoffw0 commented Dec 15, 2021

Looks like we lose a (false positive) result for cpp/comparison-of-identical-expressions: https://lgtm.com/query/5961269154262927431/ 👍

Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

All good.

@geoffw0 geoffw0 merged commit 9363d64 into github:main Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LGTM.com - false positive cpp/comparison-of-identical-expressions
2 participants