[PATCH] D31713: [Basic] getColumnNumber returns location of CR+LF on Windows

2017-04-06 Thread Chih-Hung Hsieh via Phabricator via cfe-commits 
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=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

2017-04-06 Thread Chih-Hung Hsieh via Phabricator via cfe-commits 
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

2017-04-06 Thread Alexander Kornienko via Phabricator via cfe-commits 
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

2017-04-05 Thread Chih-Hung Hsieh via Phabricator via cfe-commits 
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