Skip to content

Conversation

@jbj
Copy link
Contributor

@jbj jbj commented Jun 5, 2019

This query is not producing good enough results to justify @precision high. It fundamentally looks for a pattern that should correlate with memory management errors, but it doesn't look for the errors themselves.

I've wanted to take this query off LGTM for a long time. This forum post and the following Slack discussion is the occasion for doing it now.

@jbj jbj added the C++ label Jun 5, 2019
@jbj jbj requested a review from geoffw0 June 5, 2019 03:17
@jbj jbj requested a review from a team as a code owner June 5, 2019 03:17
geoffw0
geoffw0 previously approved these changes Jun 5, 2019
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'll give other people one last chance to comment before merging.

@geoffw0
Copy link
Contributor

geoffw0 commented Jun 5, 2019

And there's a merge conflict now...

@zlaski-semmle
Copy link
Contributor

Perhaps the change notes should indicate why the precision is being reduced?

This query is not producing good enough results to justify `@precision
high`. It's fundamentally looking for a pattern that should correlate
with memory management errors, but it doesn't look for the errors
themselves.
@jbj jbj force-pushed the suspicious-pointer-scaling_medium branch from 5e2bbf3 to cf96035 Compare June 7, 2019 04:09
@jbj
Copy link
Contributor Author

jbj commented Jun 7, 2019

I've fixed the conflict and added justification to the change note. The wording is taken from how cpp/constructor-used-as-copy-constructor is described in #1413.

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.

👍

@geoffw0 geoffw0 merged commit ab507aa into github:master Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants