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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
