[PATCH] D15411: [clang-tidy] Check for suspicious string assignments.

Gábor Horváth via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 14 06:26:53 PST 2015


xazax.hun added inline comments.

================
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:20
@@ +19,3 @@
+AST_MATCHER(QualType, isAnyCharacter) { return Node->isAnyCharacterType(); }
+} // namespace
+
----------------
aaron.ballman wrote:
> alexfh wrote:
> > I prefer `isAnyCharacter` for consistency with `isInteger`. I'd also suggest to move the matcher to ASTMatchers.h eventually.
> > I prefer isAnyCharacter for consistency with isInteger. I'd also suggest to move the matcher to ASTMatchers.h eventually.
> 
> (CCing in Manuel for his opinion as well.)
> 
> I like the consistency idea, but it was my understanding that we want all AST matcher functions to match the C++ API they are exposing, which is why I still have a slight preference for isAnyCharacterType().
Adding this checker to ASTMatchers.h seems like a trivial modification. I think I will just add it and alter this patch once this check is accepted and you decided how to call it.

================
Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:26
@@ +25,3 @@
+int main() {
+  std::string s{"foobar"};
+  std::wstring ws{L"foobar"};
----------------
alexfh wrote:
> Why do you need to initialize the strings?
I do not need to initialize them. I made the tests more minimal.


http://reviews.llvm.org/D15411





More information about the cfe-commits mailing list