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 need to be escaped and then 
> further processing happens, but you check on identity to `'` and return the 
> escaped version of it. 
> That confuses me.
Does the new comment clear it up?


================
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:54
+  // Now replace the " with '.
+  auto Pos = Result.find_first_of('"');
+  if (Pos == Result.npos)
----------------
JonasToth wrote:
> This expects at max 2 `"` characters. couldnt there be more?
The only way this will be run is if the string is a single character, so the 
only possibility if there are more than 2 " characters is that the character 
that is the delimiter is actually " which is taken care of in the check. Does 
that make sense/do you think I need to add a comment about this?


================
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:75
+
+  // string_view passed by value and contructed from string literal.
+  auto StringViewArg =
----------------
JonasToth wrote:
> What about the `std::string_view`?
This matcher is only here for the next matcher to check to see if 
absl::ByAnyChar has been passed in a single character string view and is only 
necessary because ByAnyChar accepts an absl::string_view so it isn't necessary 
to make one for std::string_view


================
Comment at: test/clang-tidy/abseil-faster-strsplit-delimiter.cpp:42
+void SplitDelimiters() {
+  absl::StrSplit("ABC", "A");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: 
absl::StrSplit()/ByAnyChar()/MaxSplits() called with a string literal 
consisting of a single character; consider using the more efficient overload 
accepting a character [abseil-faster-strsplit-delimiter]
----------------
JonasToth wrote:
> Please add a test, where `"A"` is used as an arbitray function argument 
> (similar to the template case, but without templates involved)
Can you explain this more/provide an example? 


https://reviews.llvm.org/D50862



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to