[PATCH] D45932: [clang-tidy][modernize-raw-string-literal] Don't replace upper ASCII with raw literals

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added inline comments.


================
Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:71
+  // supported by specific code pages only.
+  if (Bytes.find_if_not(isASCII) != StringRef::npos)
+    return false;
----------------
aaron.ballman wrote:
> zinovy.nis wrote:
> > aaron.ballman wrote:
> > > zinovy.nis wrote:
> > > > aaron.ballman wrote:
> > > > > I am starting to think that this functionality should be refactored because the check is now O(N^2) in the worst case because all of the bytes of the string need to be touched twice. It would be nice for performance reasons to combine this so that there's only a single pass over all of the characters.
> > > > > 
> > > > > What do you think?
> > > > Sorry, but why O(N^2)? `isASCII` is O(1), it's just `return C<=127`. `find_if_not` is O(N).
> > > The `find_if_not()` call you add touches every character in the string to see if it's ASCII and it all characters are ASCII, then `containsEscape()` calls `find()` which touches every character again (assuming the searched character is never encountered).
> > > 
> > > Looking a bit deeper, `isRawStringLiteral()` also calls `find()`, but it asserts that the character is found, so not every character is touched in the string and it should find a the searched character quite quickly.
> > OK, I'll see how to combine theses checks into a single one.
> > 
> > But anyway I see only 2*O(N), not O(N^2) here.
> Oh, derp, that's my thinko -- sorry! You are correct, that's 2 * O(N) and not O(N^2).
May be we can use here [[ http://en.cppreference.com/w/cpp/string/byte/isprint | std::isprint ]]


https://reviews.llvm.org/D45932