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

Deanna Garcia via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 20 06:39:40 PDT 2018


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:
> > > 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?
> Ok I see, you could add an assertion before this section. Having the precondition checked is always a good thing and usually adds clarity as well :)
I can't think of a simple thing to assert because most literals will look like: "a" but there is also the possibility that it looks like "\"" and I can't think of an easy way to combine those two. Do you have an idea/maybe I could just put a comment at the beginning saying this is only meant for single character string literals?


https://reviews.llvm.org/D50862





More information about the cfe-commits mailing list