[PATCH] D68227: [clang-format] [PR43372] - clang-format shows replacements in DOS files when no replacement is needed

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:1828
 
+// Replace "\r\n" with "\n"
+std::string replaceCRLF(std::string Code) {
----------------
nit: missing trailing fullstop.


================
Comment at: clang/lib/Format/Format.cpp:1832
+  while ((Pos = Code.find("\r\n", Pos)) != std::string::npos) {
+    Code.replace(Pos, 2, "\n");
+    Pos += 1;
----------------
The worst-case complexity of this code is quadratic, since `std::string::replace` is linear in the size of the resulting string.
If we have an include block of a few hundred lines, each a hundred or so characters, I could imagine that could easily add up.
I'd suggest constructing the result by appending to a fresh string to keep the overall complexity linear.
You can then take the argument by const reference.


================
Comment at: clang/lib/Format/Format.cpp:1838
+
+// Compares two strings but ignores any \r in either strings.
+static bool compareIgnoringCarriageReturns(std::string Result,
----------------
Maybe refine this comment: ignoring any `\r`, followed by `\n` in either strings.
Alternatively, since this function is just performing a comparison, consider directly inlining it in the `if` statements below (not too strong opinion).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68227/new/

https://reviews.llvm.org/D68227