Skip to content

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Jan 29, 2021

This is my long awaited solution to the problems we've been seeing with cpp/wrong-type-format-argument producing false positives in the presence of certain printf implementations (see https://lgtm.com/projects/g/microsoft/ChakraCore/alerts/?mode=tree&ruleFocus=2160310550).

As it happens this change removes ALL results on ChakraCore for cpp/wrong-type-format-argument (and also cpp/non-portable-printf, which is not on LGTM anyway). I believe that is what we want. Results are preserved on the other projects I tried (BOINC_boinc and godotengine_godot) so I believe the change is functioning as intended.

I went with exists(f.getBlock()) rather than exists(f.getFile().getRelativePath()) because, in my tests, the former would exclude things I don't think we really need it to (where the header has been copied into the source directory but the corresponding source has not?).

@geoffw0 geoffw0 added the C++ label Jan 29, 2021
@geoffw0 geoffw0 requested a review from a team as a code owner January 29, 2021 21:23
@jbj
Copy link
Contributor

jbj commented Feb 1, 2021

The tests failed.

@geoffw0
Copy link
Contributor Author

geoffw0 commented Feb 2, 2021

Explanation of the test changes:

  • the swprintf function in the test was no longer considered a FormattingFunction, because it depends on vswprintf, a vprintf variant with an implementation in the snapshot.
  • some further results were affected because getAFormatterWideType no longer recognized that the snapshot uses char16_t as a wide type at all, and falls back to the default wchar_t (this sort of thing is less likely to change in a bigger / real world snapshot).

In my opinion this effect devalues the test, which is about identifying when wide strings are char16_t, so I've fixed it by changing the test. I'm not entirely sure why the function had an implementation anyway. Happy to add further testing if anyone feels we are now missing something.

@jbj jbj merged commit aa9ab41 into github:main Feb 2, 2021
geoffw0 pushed a commit to geoffw0/ql that referenced this pull request Feb 3, 2021
C++: Exclude custom vprintf implementations from primitiveVariadicFormatter.
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.

2 participants