Review comments

Author: tbaederr

clang/lib/Frontend/TextDiagnostic.cpp

@@ -357,13 +357,13 @@ struct SourceColumnMap {
 static void selectInterestingSourceRegion(std::string &SourceLine,
                                           std::string &CaretLine,
                                           std::string &FixItInsertionLine,
-                                          unsigned NonGutterColumns,
+                                          Columns NonGutterColumns,
                                           const SourceColumnMap &map) {
   Columns CaretColumns = Columns(CaretLine.size());
   Columns FixItColumns =
       Columns(llvm::sys::locale::columnWidth(FixItInsertionLine));
-  unsigned MaxColumns =
-      std::max(map.columns().V, std::max(CaretColumns.V, FixItColumns.V));
+  Columns MaxColumns =
+      std::max({map.columns().V, CaretColumns.V, FixItColumns.V});
   // if the number of columns is less than the desired number we're done
   if (MaxColumns <= NonGutterColumns)
     return;
@@ -391,21 +391,24 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
     // We can safely use the byte offset FixItStart as the column offset
     // because the characters up until FixItStart are all ASCII whitespace
     // characters.
-    unsigned FixItStart = 0, FixItEnd = FixItInsertionLine.size();
-    for (; FixItStart != FixItEnd; ++FixItStart)
-      if (!isWhitespace(FixItInsertionLine[FixItStart]))
+    Bytes FixItStart = 0;
+    Bytes FixItEnd = Bytes(FixItInsertionLine.size());
+    for (; FixItStart != FixItEnd; FixItStart = FixItStart.next())
+      if (!isWhitespace(FixItInsertionLine[FixItStart.V]))
         break;
 
-    for (; FixItEnd != FixItStart; --FixItEnd)
-      if (!isWhitespace(FixItInsertionLine[FixItEnd - 1]))
+    for (; FixItEnd != FixItStart; FixItEnd = FixItEnd.prev())
+      if (!isWhitespace(FixItInsertionLine[FixItEnd.V - 1]))
         break;
 
-    Columns FixItStartCol = FixItStart;
-    Columns FixItEndCol = Columns(
-        llvm::sys::locale::columnWidth(FixItInsertionLine.substr(0, FixItEnd)));
+    assert(map.byteToColumn(FixItStart).V == FixItStart.V &&
+           "FixItStart should be all ASCII");
+    Columns FixItStartCol = Columns(FixItStart.V);
+    Columns FixItEndCol = Columns(llvm::sys::locale::columnWidth(
+        FixItInsertionLine.substr(0, FixItEnd.V)));
 
-    CaretStart = Columns(std::min(FixItStartCol.V, CaretStart.V));
-    CaretEnd = Columns(std::max(FixItEndCol.V, CaretEnd.V));
+    CaretStart = std::min(FixItStartCol.V, CaretStart.V);
+    CaretEnd = std::max(FixItEndCol.V, CaretEnd.V);
   }
 
   // CaretEnd may have been set at the middle of a character
@@ -427,10 +430,8 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
   // number of columns we have, try to grow the slice to encompass
   // more context.
 
-  Bytes SourceStart =
-      map.columnToByte(Columns(std::min<int>(CaretStart.V, map.columns().V)));
-  Bytes SourceEnd =
-      map.columnToByte(Columns(std::min<int>(CaretEnd.V, map.columns().V)));
+  Bytes SourceStart = map.columnToByte(std::min(CaretStart.V, map.columns().V));
+  Bytes SourceEnd = map.columnToByte(std::min(CaretEnd.V, map.columns().V));
 
   Columns CaretColumnsOutsideSource =
       CaretEnd - CaretStart -
@@ -1016,7 +1017,7 @@ static void highlightRange(const LineRange &R, const SourceColumnMap &Map,
     StartByte = Map.startOfNextColumn(StartByte);
 
   // Pick the last non-whitespace column.
-  Bytes EndByte = Bytes{std::min(R.EndByte.V, Map.bytes().V)};
+  Bytes EndByte = std::min(R.EndByte.V, Map.bytes().V);
   while (EndByte.V != 0 && (Map.getSourceLine()[EndByte.V - 1] == ' ' ||
                             Map.getSourceLine()[EndByte.V - 1] == '\t'))
     EndByte = Map.startOfPreviousColumn(EndByte);
@@ -1437,7 +1438,8 @@ void TextDiagnostic::emitSnippetAndCaret(
     // Next, insert the caret itself.
     if (CaretLineNo == LineNo) {
       Columns Col = SourceColMap.byteToContainingColumn(CaretByte.prev());
-      CaretLine.resize(std::max((size_t)Col.V + 1, CaretLine.size()), ' ');
+      CaretLine.resize(
+          std::max(static_cast<size_t>(Col.V) + 1, CaretLine.size()), ' ');
       CaretLine[Col.V] = '^';
     }
 
@@ -1446,8 +1448,8 @@ void TextDiagnostic::emitSnippetAndCaret(
 
     // If the source line is too long for our terminal, select only the
     // "interesting" source region within that line.
-    unsigned MessageLength = DiagOpts.MessageLength;
-    if (MessageLength)
+    Columns MessageLength = DiagOpts.MessageLength;
+    if (MessageLength.V != 0)
       selectInterestingSourceRegion(SourceLine, CaretLine, FixItInsertionLine,
                                     MessageLength, SourceColMap);