[PATCH] D31713: [Basic] getColumnNumber returns location of CR+LF on Windows
This revision was automatically updated to reflect the committed changes. Closed by commit rL299681: [Basic] getColumnNumber returns location of CR+LF on Windows (authored by chh). Changed prior to commit: https://reviews.llvm.org/D31713?vs=94402&id=94410#toc Repository: rL LLVM https://reviews.llvm.org/D31713 Files: cfe/trunk/lib/Basic/SourceManager.cpp Index: cfe/trunk/lib/Basic/SourceManager.cpp === --- cfe/trunk/lib/Basic/SourceManager.cpp +++ cfe/trunk/lib/Basic/SourceManager.cpp @@ -1136,19 +1136,28 @@ return 1; } + const char *Buf = MemBuf->getBufferStart(); // See if we just calculated the line number for this FilePos and can use // that to lookup the start of the line instead of searching for it. if (LastLineNoFileIDQuery == FID && LastLineNoContentCache->SourceLineCache != nullptr && LastLineNoResult < LastLineNoContentCache->NumLines) { unsigned *SourceLineCache = LastLineNoContentCache->SourceLineCache; unsigned LineStart = SourceLineCache[LastLineNoResult - 1]; unsigned LineEnd = SourceLineCache[LastLineNoResult]; -if (FilePos >= LineStart && FilePos < LineEnd) +if (FilePos >= LineStart && FilePos < LineEnd) { + // LineEnd is the LineStart of the next line. + // A line ends with separator LF or CR+LF on Windows. + // FilePos might point to the last separator, + // but we need a column number at most 1 + the last column. + if (FilePos + 1 == LineEnd && FilePos > LineStart) { +if (Buf[FilePos - 1] == '\r' || Buf[FilePos - 1] == '\n') + --FilePos; + } return FilePos - LineStart + 1; +} } - const char *Buf = MemBuf->getBufferStart(); unsigned LineStart = FilePos; while (LineStart && Buf[LineStart-1] != '\n' && Buf[LineStart-1] != '\r') --LineStart; Index: cfe/trunk/lib/Basic/SourceManager.cpp === --- cfe/trunk/lib/Basic/SourceManager.cpp +++ cfe/trunk/lib/Basic/SourceManager.cpp @@ -1136,19 +1136,28 @@ return 1; } + const char *Buf = MemBuf->getBufferStart(); // See if we just calculated the line number for this FilePos and can use // that to lookup the start of the line instead of searching for it. if (LastLineNoFileIDQuery == FID && LastLineNoContentCache->SourceLineCache != nullptr && LastLineNoResult < LastLineNoContentCache->NumLines) { unsigned *SourceLineCache = LastLineNoContentCache->SourceLineCache; unsigned LineStart = SourceLineCache[LastLineNoResult - 1]; unsigned LineEnd = SourceLineCache[LastLineNoResult]; -if (FilePos >= LineStart && FilePos < LineEnd) +if (FilePos >= LineStart && FilePos < LineEnd) { + // LineEnd is the LineStart of the next line. + // A line ends with separator LF or CR+LF on Windows. + // FilePos might point to the last separator, + // but we need a column number at most 1 + the last column. + if (FilePos + 1 == LineEnd && FilePos > LineStart) { +if (Buf[FilePos - 1] == '\r' || Buf[FilePos - 1] == '\n') + --FilePos; + } return FilePos - LineStart + 1; +} } - const char *Buf = MemBuf->getBufferStart(); unsigned LineStart = FilePos; while (LineStart && Buf[LineStart-1] != '\n' && Buf[LineStart-1] != '\r') --LineStart; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31713: [Basic] getColumnNumber returns location of CR+LF on Windows
chh updated this revision to Diff 94402. chh marked an inline comment as done. https://reviews.llvm.org/D31713 Files: lib/Basic/SourceManager.cpp Index: lib/Basic/SourceManager.cpp === --- lib/Basic/SourceManager.cpp +++ lib/Basic/SourceManager.cpp @@ -1136,19 +1136,28 @@ return 1; } + const char *Buf = MemBuf->getBufferStart(); // See if we just calculated the line number for this FilePos and can use // that to lookup the start of the line instead of searching for it. if (LastLineNoFileIDQuery == FID && LastLineNoContentCache->SourceLineCache != nullptr && LastLineNoResult < LastLineNoContentCache->NumLines) { unsigned *SourceLineCache = LastLineNoContentCache->SourceLineCache; unsigned LineStart = SourceLineCache[LastLineNoResult - 1]; unsigned LineEnd = SourceLineCache[LastLineNoResult]; -if (FilePos >= LineStart && FilePos < LineEnd) +if (FilePos >= LineStart && FilePos < LineEnd) { + // LineEnd is the LineStart of the next line. + // A line ends with separator LF or CR+LF on Windows. + // FilePos might point to the last separator, + // but we need a column number at most 1 + the last column. + if (FilePos + 1 == LineEnd && FilePos > LineStart) { +if (Buf[FilePos - 1] == '\r' || Buf[FilePos - 1] == '\n') + --FilePos; + } return FilePos - LineStart + 1; +} } - const char *Buf = MemBuf->getBufferStart(); unsigned LineStart = FilePos; while (LineStart && Buf[LineStart-1] != '\n' && Buf[LineStart-1] != '\r') --LineStart; Index: lib/Basic/SourceManager.cpp === --- lib/Basic/SourceManager.cpp +++ lib/Basic/SourceManager.cpp @@ -1136,19 +1136,28 @@ return 1; } + const char *Buf = MemBuf->getBufferStart(); // See if we just calculated the line number for this FilePos and can use // that to lookup the start of the line instead of searching for it. if (LastLineNoFileIDQuery == FID && LastLineNoContentCache->SourceLineCache != nullptr && LastLineNoResult < LastLineNoContentCache->NumLines) { unsigned *SourceLineCache = LastLineNoContentCache->SourceLineCache; unsigned LineStart = SourceLineCache[LastLineNoResult - 1]; unsigned LineEnd = SourceLineCache[LastLineNoResult]; -if (FilePos >= LineStart && FilePos < LineEnd) +if (FilePos >= LineStart && FilePos < LineEnd) { + // LineEnd is the LineStart of the next line. + // A line ends with separator LF or CR+LF on Windows. + // FilePos might point to the last separator, + // but we need a column number at most 1 + the last column. + if (FilePos + 1 == LineEnd && FilePos > LineStart) { +if (Buf[FilePos - 1] == '\r' || Buf[FilePos - 1] == '\n') + --FilePos; + } return FilePos - LineStart + 1; +} } - const char *Buf = MemBuf->getBufferStart(); unsigned LineStart = FilePos; while (LineStart && Buf[LineStart-1] != '\n' && Buf[LineStart-1] != '\r') --LineStart; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31713: [Basic] getColumnNumber returns location of CR+LF on Windows
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with one nit. Thank you for tracking down this bug and fixing it! Comment at: lib/Basic/SourceManager.cpp:1153 + if (FilePos + 1 == LineEnd && FilePos > LineStart) { +const char *Buf = MemBuf->getBufferStart(); +if (Buf[FilePos - 1] == '\r' || Buf[FilePos - 1] == '\n') I'd move this line before the outermost `if` and remove the identical line below. Repository: rL LLVM https://reviews.llvm.org/D31713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31713: [Basic] getColumnNumber returns location of CR+LF on Windows
chh created this revision. When fixing a Clang-Tidy bug in https://reviews.llvm.org/D31406, http://bugs.llvm.org/show_bug.cgi?id=32402, reuse of FileID enabled the missing highlightRange function. Assertion in highlightRange failed because the end-of-range column number was 2 + the last column of a line on Windows. This fix is required to enable https://reviews.llvm.org/D31406. Repository: rL LLVM https://reviews.llvm.org/D31713 Files: lib/Basic/SourceManager.cpp Index: lib/Basic/SourceManager.cpp === --- lib/Basic/SourceManager.cpp +++ lib/Basic/SourceManager.cpp @@ -1144,8 +1144,18 @@ unsigned *SourceLineCache = LastLineNoContentCache->SourceLineCache; unsigned LineStart = SourceLineCache[LastLineNoResult - 1]; unsigned LineEnd = SourceLineCache[LastLineNoResult]; -if (FilePos >= LineStart && FilePos < LineEnd) +if (FilePos >= LineStart && FilePos < LineEnd) { + // LineEnd is the LineStart of the next line. + // A line ends with separator LF or CR+LF on Windows. + // FilePos might point to the last separator, + // but we need a column number at most 1 + the last column. + if (FilePos + 1 == LineEnd && FilePos > LineStart) { +const char *Buf = MemBuf->getBufferStart(); +if (Buf[FilePos - 1] == '\r' || Buf[FilePos - 1] == '\n') + --FilePos; + } return FilePos - LineStart + 1; +} } const char *Buf = MemBuf->getBufferStart(); Index: lib/Basic/SourceManager.cpp === --- lib/Basic/SourceManager.cpp +++ lib/Basic/SourceManager.cpp @@ -1144,8 +1144,18 @@ unsigned *SourceLineCache = LastLineNoContentCache->SourceLineCache; unsigned LineStart = SourceLineCache[LastLineNoResult - 1]; unsigned LineEnd = SourceLineCache[LastLineNoResult]; -if (FilePos >= LineStart && FilePos < LineEnd) +if (FilePos >= LineStart && FilePos < LineEnd) { + // LineEnd is the LineStart of the next line. + // A line ends with separator LF or CR+LF on Windows. + // FilePos might point to the last separator, + // but we need a column number at most 1 + the last column. + if (FilePos + 1 == LineEnd && FilePos > LineStart) { +const char *Buf = MemBuf->getBufferStart(); +if (Buf[FilePos - 1] == '\r' || Buf[FilePos - 1] == '\n') + --FilePos; + } return FilePos - LineStart + 1; +} } const char *Buf = MemBuf->getBufferStart(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits