Szelethus added a comment.
I'm only a beginner, but here are some things that caught my eye. I really like
the idea! :)
================
Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:28
+
+// PointerSortingVisitor class.
+class PointerSortingVisitor : public StmtVisitor<PointerSortingVisitor> {
----------------
This comment holds little value.
================
Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:46
+};
+}
+
----------------
` // end of anonymous namespace`
================
Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:88
+
+ if (!II->getName().equals("sort"))
+ return;
----------------
whisperity wrote:
> Brrr... `equals`. StringRef has a `==` and `!=` operator which accepts string
> literals on the other side, which would result in a more concise code.
>
> Also, this heuristic can be applied without extra changes (apart from those
> mentioned by NoQ and might be mentioned later by others) to other sorting
> functions, such as `std::stable_sort` and `std::stable_partition`. Perhaps it
> would be worthy to enable checking those functions too.
Maybe `II->isStr("sort")`?
================
Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:93
+ const RecordDecl *RD =
+ IterTy->getUnqualifiedDesugaredType()->getAsCXXRecordDecl();
+
----------------
Is there a reason for not directly acquiring the record declaration?
================
Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:119
+};
+}
+
----------------
` // end of anonymous namespace`
================
Comment at: test/Analysis/ptr-sort.cpp:1
+// RUN: %clang_analyze_cc1
-analyzer-checker=alpha.nondeterminism.PointerSorting %s -analyzer-output=text
-verify
+
----------------
Always run the core checkers too.
`-analyzer-checker=core,alpha.nondeterminism.PointerSorting`
Repository:
rC Clang
https://reviews.llvm.org/D50488
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
