[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

hokein closed this revision. hokein added a comment. Committed in https://reviews.llvm.org/rL340411. https://reviews.llvm.org/D50862 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

JonasToth added a comment. LGTM https://reviews.llvm.org/D50862 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. The patch looks good. I'll commit for you once @JonasToth has no further comments. https://reviews.llvm.org/D50862 ___ cfe-commits mailing list

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

deannagarcia updated this revision to Diff 161513. https://reviews.llvm.org/D50862 Files: clang-tidy/abseil/AbseilTidyModule.cpp clang-tidy/abseil/CMakeLists.txt clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp clang-tidy/abseil/FasterStrsplitDelimiterCheck.h docs/ReleaseNotes.rst d

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

JonasToth added inline comments. Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:54 + // Now replace the " with '. + auto Pos = Result.find_first_of('"'); + if (Pos == Result.npos) deannagarcia wrote: > JonasToth wrote: > > deannagarcia wrote: >

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

deannagarcia added inline comments. Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:54 + // Now replace the " with '. + auto Pos = Result.find_first_of('"'); + if (Pos == Result.npos) JonasToth wrote: > deannagarcia wrote: > > JonasToth wrote: >

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

deannagarcia updated this revision to Diff 161472. deannagarcia marked 11 inline comments as done. https://reviews.llvm.org/D50862 Files: clang-tidy/abseil/AbseilTidyModule.cpp clang-tidy/abseil/CMakeLists.txt clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp clang-tidy/abseil/FasterStrs

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

JonasToth added inline comments. Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:85 + + // absl::StrSplit(..., "x") -> absl::StrSplit(..., 'x') + // absl::ByAnyChar("x") -> 'x' Maybe you could make these comments into full sentences. I do think

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

deannagarcia added inline comments. Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:49 + // in the character literal. + if (Result == R"("'")") { +return std::string(R"('\'')"); JonasToth wrote: > The comment suggest, that all single quotes n

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

deannagarcia updated this revision to Diff 161309. deannagarcia marked 10 inline comments as done. https://reviews.llvm.org/D50862 Files: clang-tidy/abseil/AbseilTidyModule.cpp clang-tidy/abseil/CMakeLists.txt clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp clang-tidy/abseil/FasterStrs

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

hokein added a subscriber: sbenza. hokein added a comment. Thanks for porting the check to upstream (context: the check was developed internally, and has been run on our codebase for a while, it is pretty stable). Could you please update the patch message to indicate this is a porting change, a

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst:6 + +This check triggers on calls to ``absl::StrSplit()`` or ``absl::MaxSplits()`` +where the delimiter is a single character string literal. The check will offer ---

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

JonasToth added inline comments. Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:24 + +ast_matchers::internal::Matcher +constructExprWithArg(llvm::StringRef ClassName, you dont need the `ast_matchers` namespace as there is a `using namespace ast_m

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

deannagarcia created this revision. deannagarcia added reviewers: hokein, alexfh. deannagarcia added a project: clang-tools-extra. Herald added subscribers: xazax.hun, mgorny. This check is an abseil specific check that checks for code using single character string literals as delimiters and tran