Frontend: Fix SourceColumnMap assertion failure on non-ascii characters.

[rsmith]

LGTM, thank you! Sorry for the delay.

test/Frontend/source-col-map.c

5–16

The comment here should explain what we're testing, not talk about the structure of the code under test -- that code might change, but this comment should not need updating when it does. This is an excellent writeup for the commit message, though.

[logan]

Thanks! Committed as rL225442 with revised commit message and test case comments.

lib/Frontend/TextDiagnostic.cpp

struct SourceColumnMap {
   /// multi-column entity.
   int columnToByte(int n) const {
     assert(0<=n && n<static_cast<int>(m_columnToByte.size()));
     return m_columnToByte[n];
   }
 
   /// \brief Map from a byte index to the next byte which starts a column.
   int startOfNextColumn(int N) const {
-    assert(0 <= N && N < static_cast<int>(m_columnToByte.size() - 1));
+    assert(0 <= N && N < static_cast<int>(m_byteToColumn.size() - 1));
     while (byteToColumn(++N) == -1) {}
     return N;
   }
 
   /// \brief Map from a byte index to the previous byte which starts a column.
   int startOfPreviousColumn(int N) const {
-    assert(0 < N && N < static_cast<int>(m_columnToByte.size()));
+    assert(0 < N && N < static_cast<int>(m_byteToColumn.size()));
     while (byteToColumn(--N) == -1) {}
     return N;
   }
 
   StringRef getSourceLine() const {
     return m_SourceLine;
   }
   
 private:
   const std::string m_SourceLine;
   SmallVector<int,200> m_byteToColumn;
   SmallVector<int,200> m_columnToByte;
 };
 } // end anonymous namespace
 
 /// \brief When the source code line we want to print is too long for
 /// the terminal, select the "interesting" region.
 static void selectInterestingSourceRegion(std::string &SourceLine,
                                           std::string &CaretLine,
                                           std::string &FixItInsertionLine,
                                           unsigned Columns,
                                           const SourceColumnMap &map) {
-  unsigned MaxColumns = std::max<unsigned>(map.columns(),
-                                           std::max(CaretLine.size(),
-                                                    FixItInsertionLine.size()));
+  unsigned CaretColumns = CaretLine.size();
+  unsigned FixItColumns = llvm::sys::locale::columnWidth(FixItInsertionLine);
+  unsigned MaxColumns = std::max(static_cast<unsigned>(map.columns()),
+                                 std::max(CaretColumns, FixItColumns));
   // if the number of columns is less than the desired number we're done
   if (MaxColumns <= Columns)
     return;
 
   // No special characters are allowed in CaretLine.
   assert(CaretLine.end() ==
          std::find_if(CaretLine.begin(), CaretLine.end(),
                       [](char c) { return c < ' ' || '~' < c; }));
void TextDiagnostic::emitSnippetAndCaret(
 
   // Arbitrarily stop showing snippets when the line is too long.
   if (size_t(LineEnd - LineStart) > MaxLineLengthToPrint)
     return;
 
   // Copy the line of code into an std::string for ease of manipulation.
   std::string SourceLine(LineStart, LineEnd);
 
-  // Create a line for the caret that is filled with spaces that is the same
-  // length as the line of source code.
-  std::string CaretLine(LineEnd-LineStart, ' ');
-
+  // Build the byte to column map.
   const SourceColumnMap sourceColMap(SourceLine, DiagOpts->TabStop);
 
+  // Create a line for the caret that is filled with spaces that is the same
+  // number of columns as the line of source code.
+  std::string CaretLine(sourceColMap.columns(), ' ');
+
   // Highlight all of the characters covered by Ranges with ~ characters.
   for (SmallVectorImpl<CharSourceRange>::iterator I = Ranges.begin(),
                                                   E = Ranges.end();
        I != E; ++I)
     highlightRange(*I, LineNo, FID, sourceColMap, CaretLine, SM, LangOpts);
 
   // Next, insert the caret itself.
   ColNo = sourceColMap.byteToContainingColumn(ColNo-1);

test/Frontend/source-col-map.c

+// RUN: not %clang_cc1 %s -fsyntax-only -fmessage-length 75 -o /dev/null 2>&1 | FileCheck %s -strict-whitespace
+
+// Test case for the text diagnostics source column conversion crash.
+
+// There are two problems to be tested:
+//
+// 1. The assertion in startOfNextColumn() and startOfPreviousColumn()
+//    should not be raised when the byte index is greater than the column index
+//    since the non-ascii characters may use more than one bytes to store a
+//    character in a column.
+//
+// 2. The length of the caret line should be equal to the number of columns of
+//    source line, instead of the length of the source line.  Otherwise, the
+//    assertion in selectInterestingSourceRegion will be raised because the
+//    removed columns plus the kept columns are not greater than the max column,
+//    which means that we should not remove any column at all.
+
+// NOTE: This file is encoded in UTF-8 and intentionally contains some
+// non-ASCII characters.
+
+__attribute__((format(printf, 1, 2)))
+extern int printf(const char *fmt, ...);
+
+void test1(Unknown* b);  // αααα αααα αααα αααα αααα αααα αααα αααα αααα αααα αααα
+// CHECK: unknown type name 'Unknown'
+// CHECK-NEXT: void test1(Unknown* b);  // αααα αααα αααα αααα αααα αααα αααα ααα...
+// CHECK-NEXT: {{^           \^$}}
+
+void test2(Unknown* b);  // αααα αααα αααα αααα αααα αααα αααα αααα αααα
+
+// CHECK: unknown type name 'Unknown'
+// CHECK-NEXT: void test2(Unknown* b);  // αααα αααα αααα αααα αααα αααα αααα αααα αααα
+// CHECK-NEXT: {{^           \^$}}
+
+void test3() {
+   /* αααα αααα αααα αααα αααα αααα αααα αααα αααα αααα */ printf("%d", "s");
+}
+// CHECK:       format specifies type 'int' but the argument has type 'char *'
+// CHECK-NEXT:   ...αααα αααα αααα αααα αααα αααα αααα αααα αααα */ printf("%d", "s");
+// CHECK-NEXT: {{^                                                             ~~   \^~~$}}
+// CHECK-NEXT: {{^                                                             %s$}}