Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

readability-redundant-string-cstr doesn't catch c_str() calls in make_pair #24823

Open Quuxplusone opened 9 years ago

Quuxplusone commented 9 years ago
Bugzilla Link PR24824
Status NEW
Importance P normal
Reported by Eugene Zelenko (eugene.zelenko@gmail.com)
Reported on 2015-09-15 13:11:24 -0700
Last modified on 2016-01-18 23:14:17 -0800
Version unspecified
Hardware PC Linux
CC adrian@adek.io, alexfh@google.com, djasper@google.com, klimek@google.com
Fixed by commit(s)
Attachments AST (7126 bytes, application/octet-stream)
Blocks
Blocked by
See also
Created attachment 14882
AST generated from my code

readability-redundant-string-cstr doesn't catch c_str() calls in make_pair.

Sample situation:

std::map<std::string, int> Map;
std::string Key( "Key" );

Map.insert( std::make_pair( Key.c_str(), 1 ) );

I use GCC 4.8.3 STL. I tried both 3.7 and trunk clang-tidy.
Quuxplusone commented 9 years ago

Attached AST (7126 bytes, application/octet-stream): AST generated from my code

Quuxplusone commented 8 years ago
From what I can tell after reading the code of this check, the way matchers are
registered implies that this is catching only "direct" mismatch between
arguments. Here, when I look at the AST, there is an implicit conversion
happening from std::pair<const char*, int> to std::pair<std::string, int> which
by no means is detected by this check.

I don't have much experience in writing this kind of semantic checks, but it
seems that it would need to match any function which expects std::string. Then,
the parameter which is of the std::string would have to be tracked down the AST
to get the origin (more than one layer, e.g. implicit conversions, temporaries,
...) of the value. This value, should be checked for a presence of .c_str()
call.

Can someone point to an existing solution which does a similar thing? I am sure
there are some more complex analysis already implemented, which might help in
shaping the solution.