Created attachment 15621 [details] short file Hi guys, I been recently playing with clang-tidy, and I don't know why but some checks seems to not check for what they should. I took simple example from docs (included file). I am sure I am running right checks, because list checks say so.
Created attachment 15622 [details] preprocessed file
What standard library do you use? With the version of libstdc++ I have on my machine, this seems to work (with the preprocessed file it doesn't): $ cat /tmp/qq.cc #include <string> int main() { std::string s; int x = 5965; s = 6; s = x; } $ clang-tidy -checks=-*,misc-string-integer-assignment /tmp/qq.cc -- 2 warnings generated. /tmp/qq.cc:6:9: warning: an integer is interpreted as a character code when assigning it to a string; if this is intended, cast the integer to the appropriate character type; if you want a string representation, use the appropriate conversion facility [misc-string-integer-assignment] s = 6; ^ '' /tmp/qq.cc:7:9: warning: an integer is interpreted as a character code when assigning it to a string; if this is intended, cast the integer to the appropriate character type; if you want a string representation, use the appropriate conversion facility [misc-string-integer-assignment] s = x; ^
Seems to be a problem with inline namespace __cxx11 {} again. Is this exactly the problem that http://reviews.llvm.org/D15506 is meant to solve?
(In reply to comment #3) > Seems to be a problem with inline namespace __cxx11 {} again. Is this > exactly the problem that http://reviews.llvm.org/D15506 is meant to solve? I think so.
May I ask you to apply that patch and see whether it fixes the issue?
(In reply to comment #5) > May I ask you to apply that patch and see whether it fixes the issue? Sure! I can confirm that after applying the patch from http://reviews.llvm.org/D15506 the checker can find the two issues in the attached preprocessed file.
Looking into this patch: > Allow hasName() to look through inline namespaces. This seems to be quite unexpected behaviour. Isn't just applying regular expressions via using matchesName instead of hasName while leaving hasName with more intuitive behaviour?
(In reply to comment #7) > Looking into this patch: > > > Allow hasName() to look through inline namespaces. > > This seems to be quite unexpected behaviour. Isn't just applying regular > expressions via using matchesName instead of hasName while leaving hasName > with more intuitive behaviour? Inline namespaces are often implementation details. I don't think that the checkers should deal with them unless they really want to. And I think it is unreasonable to expect the checker authors to test their checkers with all of the major standard libraries to check whether their regexp is correct for all of them.
(In reply to comment #8) > (In reply to comment #7) > > Looking into this patch: > > > > > Allow hasName() to look through inline namespaces. > > > > This seems to be quite unexpected behaviour. Isn't just applying regular > > expressions via using matchesName instead of hasName while leaving hasName > > with more intuitive behaviour? > > Inline namespaces are often implementation details. I don't think that the > checkers should deal with them unless they really want to. And I think it is > unreasonable to expect the checker authors to test their checkers with all > of the major standard libraries to check whether their regexp is correct for > all of them. Makes sense. I was about to suggest creating a custom matcher, say, hasNameThroughNamespaces or something less monstrous, but I can't find a scenario, when somebody will want to use the hasName with exact inline etc namespaces matching.
(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > Looking into this patch: > > > > > > > Allow hasName() to look through inline namespaces. > > > > > > This seems to be quite unexpected behaviour. Isn't just applying regular > > > expressions via using matchesName instead of hasName while leaving hasName > > > with more intuitive behaviour? > > > > Inline namespaces are often implementation details. I don't think that the > > checkers should deal with them unless they really want to. And I think it is > > unreasonable to expect the checker authors to test their checkers with all > > of the major standard libraries to check whether their regexp is correct for > > all of them. > > Makes sense. > > I was about to suggest creating a custom matcher, say, > hasNameThroughNamespaces or something less monstrous, but I can't find a > scenario, when somebody will want to use the hasName with exact inline etc > namespaces matching. I can think of one scenario, to warn when a deprecated version of an API is used, that is behind an inline namespace, and the library authors did not add the deprecated attribute to the API. But this is probably very rare.
Thanks guys, I am waiting till this patch will be upstream.
What is the status of it? (In reply to comment #11) > Thanks guys, I am waiting till this patch will be upstream.
Thanks for fixing!