<div dir="ltr">Looks good! I don't known if there's a better way to figure out if the comparers used are compatible.<div><br></div><div>I've seen one case where a weaker comparison is used in the std::equal_range method than in the corresponding container, but this seems to be rather suspicious on its own, so a // NOLINT with an explanation wouldn't hurt in this case.<div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 17, 2015 at 10:45 PM, Gabor Horvath <span dir="ltr"><<a href="mailto:xazax.hun@gmail.com" target="_blank">xazax.hun@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: xazax<br>
Date: Tue Feb 17 15:45:38 2015<br>
New Revision: 229552<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=229552&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=229552&view=rev</a><br>
Log:<br>
[clang-tidy] Fixed two wrong fix-it cases in misc-inefficient-algorithm checker.<br>
<br>
Modified:<br>
clang-tools-extra/trunk/clang-tidy/misc/InefficientAlgorithmCheck.cpp<br>
clang-tools-extra/trunk/test/clang-tidy/misc-inefficient-algorithm.cpp<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/misc/InefficientAlgorithmCheck.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/InefficientAlgorithmCheck.cpp?rev=229552&r1=229551&r2=229552&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/InefficientAlgorithmCheck.cpp?rev=229552&r1=229551&r2=229552&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/misc/InefficientAlgorithmCheck.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/misc/InefficientAlgorithmCheck.cpp Tue Feb 17 15:45:38 2015<br>
@@ -17,9 +17,18 @@ using namespace clang::ast_matchers;<br>
namespace clang {<br>
namespace tidy {<br>
<br>
+static bool areTypesCompatible(QualType Left, QualType Right) {<br>
+ if (const auto *LeftRefType = Left->getAs<ReferenceType>())<br>
+ Left = LeftRefType->getPointeeType();<br>
+ if (const auto *RightRefType = Right->getAs<ReferenceType>())<br>
+ Right = RightRefType->getPointeeType();<br>
+ return Left->getCanonicalTypeUnqualified() ==<br>
+ Right->getCanonicalTypeUnqualified();<br>
+}<br>
+<br>
void InefficientAlgorithmCheck::registerMatchers(MatchFinder *Finder) {<br>
const std::string Algorithms =<br>
- "^::std::(find|count|equal_range|lower_blound|upper_bound)$";<br>
+ "^::std::(find|count|equal_range|lower_bound|upper_bound)$";<br>
const auto ContainerMatcher = classTemplateSpecializationDecl(<br>
matchesName("^::std::(unordered_)?(multi)?(set|map)$"));<br>
const auto Matcher =<br>
@@ -57,6 +66,14 @@ void InefficientAlgorithmCheck::check(co<br>
const llvm::StringRef IneffContName = IneffCont->getName();<br>
const bool Unordered =<br>
IneffContName.find("unordered") != llvm::StringRef::npos;<br>
+ const bool Maplike = IneffContName.find("map") != llvm::StringRef::npos;<br>
+<br>
+ // Store if the key type of the container is compatible with the value<br>
+ // that is searched for.<br>
+ QualType ValueType = AlgCall->getArg(2)->getType();<br>
+ QualType KeyType =<br>
+ IneffCont->getTemplateArgs()[0].getAsType().getCanonicalType();<br>
+ const bool CompatibleTypes = areTypesCompatible(KeyType, ValueType);<br>
<br>
// Check if the comparison type for the algorithm and the container matches.<br>
if (AlgCall->getNumArgs() == 4 && !Unordered) {<br>
@@ -87,7 +104,7 @@ void InefficientAlgorithmCheck::check(co<br>
const auto *IneffContExpr = Result.Nodes.getNodeAs<Expr>("IneffContExpr");<br>
FixItHint Hint;<br>
<br>
- if (!AlgCall->getLocStart().isMacroID()) {<br>
+ if (!AlgCall->getLocStart().isMacroID() && !Maplike && CompatibleTypes) {<br>
std::string ReplacementText =<br>
(llvm::Twine(Lexer::getSourceText(<br>
CharSourceRange::getTokenRange(IneffContExpr->getSourceRange()),<br>
<br>
Modified: clang-tools-extra/trunk/test/clang-tidy/misc-inefficient-algorithm.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-inefficient-algorithm.cpp?rev=229552&r1=229551&r2=229552&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-inefficient-algorithm.cpp?rev=229552&r1=229551&r2=229552&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/clang-tidy/misc-inefficient-algorithm.cpp (original)<br>
+++ clang-tools-extra/trunk/test/clang-tidy/misc-inefficient-algorithm.cpp Tue Feb 17 15:45:38 2015<br>
@@ -23,6 +23,19 @@ template <typename K, typename Cmp = les<br>
iterator end() const;<br>
};<br>
<br>
+struct other_iterator_type {};<br>
+<br>
+template <typename K, typename V, typename Cmp = less<K>> struct map {<br>
+ typedef other_iterator_type iterator;<br>
+ iterator find(const K &k);<br>
+ unsigned count(const K &k);<br>
+<br>
+ iterator begin();<br>
+ iterator end();<br>
+ iterator begin() const;<br>
+ iterator end() const;<br>
+};<br>
+<br>
template <typename K> struct unordered_set : set<K> {};<br>
<br>
template <typename K, typename Cmp = less<K>> struct multiset : set<K, Cmp> {};<br>
@@ -32,15 +45,17 @@ template <typename FwIt, typename K> FwI<br>
template <typename FwIt, typename K, typename Cmp><br>
FwIt find(FwIt, FwIt, const K &, Cmp);<br>
<br>
-template <typename FwIt, typename Pred><br>
-FwIt find_if(FwIt, FwIt, Pred);<br>
+template <typename FwIt, typename Pred> FwIt find_if(FwIt, FwIt, Pred);<br>
<br>
template <typename FwIt, typename K> FwIt count(FwIt, FwIt, const K &);<br>
<br>
template <typename FwIt, typename K> FwIt lower_bound(FwIt, FwIt, const K &);<br>
+<br>
+template <typename FwIt, typename K, typename Ord><br>
+FwIt lower_bound(FwIt, FwIt, const K &, Ord);<br>
}<br>
<br>
-#define FIND_IN_SET(x) find(x.begin(), x.end(), 10)<br>
+#define FIND_IN_SET(x) find(x.begin(), x.end(), 10)<br>
// CHECK-FIXES: #define FIND_IN_SET(x) find(x.begin(), x.end(), 10)<br>
<br>
template <typename T> void f(const T &t) {<br>
@@ -93,4 +108,26 @@ int main() {<br>
find(us.begin(), us.end(), 10);<br>
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be<br>
// CHECK-FIXES: {{^ }}us.find(10);{{$}}<br>
+<br>
+ std::map<int, int> intmap;<br>
+ find(intmap.begin(), intmap.end(), 46);<br>
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be<br>
+ // CHECK-FIXES: {{^ }}find(intmap.begin(), intmap.end(), 46);{{$}}<br>
+}<br>
+<br>
+struct Value {<br>
+ int value;<br>
+};<br>
+<br>
+struct Ordering {<br>
+ bool operator()(const Value &lhs, const Value &rhs) const {<br>
+ return lhs.value < rhs.value;<br>
+ }<br>
+ bool operator()(int lhs, const Value &rhs) const { return lhs < rhs.value; }<br>
+};<br>
+<br>
+void g(std::set<Value, Ordering> container, int value) {<br>
+ lower_bound(container.begin(), container.end(), value, Ordering());<br>
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be<br>
+ // CHECK-FIXES: {{^ }}lower_bound(container.begin(), container.end(), value, Ordering());{{$}}<br>
}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br><br>
</div></div></div>