diff options
author | Paul Hoad <mydeveloperday@gmail.com> | 2019-10-01 20:20:22 +0000 |
---|---|---|
committer | Paul Hoad <mydeveloperday@gmail.com> | 2019-10-01 20:20:22 +0000 |
commit | 603650213b1aee2a03ba879ba9d8e6eb65a06694 (patch) | |
tree | 540d876fba6d243653a85ecd0a4d675042ddc0b4 /clang/lib/Format/Format.cpp | |
parent | 2df5f12ea119ea653c92fee347ef5a00ca64f0ad (diff) | |
download | bcm5719-llvm-603650213b1aee2a03ba879ba9d8e6eb65a06694.tar.gz bcm5719-llvm-603650213b1aee2a03ba879ba9d8e6eb65a06694.zip |
[clang-format] [PR43372] - clang-format shows replacements in DOS files when no replacement is needed
Summary:
This is a patch to fix PR43372 (https://bugs.llvm.org/show_bug.cgi?id=43372) - clang-format can't format file with includes, ( which really keep providing replacements for already sorted headers.)
A similar issue was addressed by @krasimir in {D60199}, however, this seemingly only prevented the issue when the files being formatted did not contain windows line endings (\r\n)
It's possible this is related to https://twitter.com/StephanTLavavej/status/1176722938243895296 given who @STL_MSFT works for!
As people often used the existence of replacements to determine if a file needs clang-formatting, this is probably pretty important for windows users
There may be a better way of comparing 2 strings and ignoring \r (which appear in both Results and Code), I couldn't choose between this idiom or the copy_if approach, but I'm happy to change it to whatever people consider more performant.
Reviewers: krasimir, klimek, owenpan, ioeric
Reviewed By: krasimir
Subscribers: cfe-commits, STL_MSFT, krasimir
Tags: #clang-format, #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D68227
llvm-svn: 373388
Diffstat (limited to 'clang/lib/Format/Format.cpp')
-rw-r--r-- | clang/lib/Format/Format.cpp | 30 |
1 files changed, 27 insertions, 3 deletions
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 07fdcc83af1..e918e90cb9f 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1825,6 +1825,28 @@ FindCursorIndex(const SmallVectorImpl<IncludeDirective> &Includes, return std::make_pair(CursorIndex, OffsetToEOL); } +// Replace all "\r\n" with "\n". +std::string replaceCRLF(const std::string &Code) { + std::string NewCode; + size_t Pos = 0, LastPos = 0; + + do { + Pos = Code.find("\r\n", LastPos); + if (Pos == LastPos) { + LastPos++; + continue; + } + if (Pos == std::string::npos) { + NewCode += Code.substr(LastPos); + break; + } + NewCode += Code.substr(LastPos, Pos - LastPos) + "\n"; + LastPos = Pos + 2; + } while (Pos != std::string::npos); + + return NewCode; +} + // Sorts and deduplicate a block of includes given by 'Includes' alphabetically // adding the necessary replacement to 'Replaces'. 'Includes' must be in strict // source order. @@ -1898,7 +1920,8 @@ static void sortCppIncludes(const FormatStyle &Style, // If the #includes are out of order, we generate a single replacement fixing // the entire range of blocks. Otherwise, no replacement is generated. - if (result == Code.substr(IncludesBeginOffset, IncludesBlockSize)) + if (replaceCRLF(result) == + replaceCRLF(Code.substr(IncludesBeginOffset, IncludesBlockSize))) return; auto Err = Replaces.add(tooling::Replacement( @@ -2066,7 +2089,8 @@ static void sortJavaImports(const FormatStyle &Style, // If the imports are out of order, we generate a single replacement fixing // the entire block. Otherwise, no replacement is generated. - if (result == Code.substr(Imports.front().Offset, ImportsBlockSize)) + if (replaceCRLF(result) == + replaceCRLF(Code.substr(Imports.front().Offset, ImportsBlockSize))) return; auto Err = Replaces.add(tooling::Replacement(FileName, Imports.front().Offset, @@ -2356,7 +2380,7 @@ reformat(const FormatStyle &Style, StringRef Code, auto Env = std::make_unique<Environment>(Code, FileName, Ranges, FirstStartColumn, - NextStartColumn, LastStartColumn); + NextStartColumn, LastStartColumn); llvm::Optional<std::string> CurrentCode = None; tooling::Replacements Fixes; unsigned Penalty = 0; |