Skip to content

Conversation

@tbaederr
Copy link
Contributor

Also rename map to Map, remove the m_ prefix from member variables and fix the naming of the existing color variables.

@tbaederr tbaederr requested a review from Sirraide October 30, 2025 08:41
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Oct 30, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 30, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Also rename map to Map, remove the m_ prefix from member variables and fix the naming of the existing color variables.


Full diff: https://github.com/llvm/llvm-project/pull/165680.diff

1 Files Affected:

  • (modified) clang/lib/Frontend/TextDiagnostic.cpp (+85-86)
diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp
index c33d8f8ca9ebd..4188b5d559f8c 100644
--- a/clang/lib/Frontend/TextDiagnostic.cpp
+++ b/clang/lib/Frontend/TextDiagnostic.cpp
@@ -22,22 +22,16 @@
 
 using namespace clang;
 
-static const enum raw_ostream::Colors noteColor = raw_ostream::CYAN;
-static const enum raw_ostream::Colors remarkColor =
-  raw_ostream::BLUE;
-static const enum raw_ostream::Colors fixitColor =
-  raw_ostream::GREEN;
-static const enum raw_ostream::Colors caretColor =
-  raw_ostream::GREEN;
-static const enum raw_ostream::Colors warningColor =
-  raw_ostream::MAGENTA;
-static const enum raw_ostream::Colors templateColor =
-  raw_ostream::CYAN;
-static const enum raw_ostream::Colors errorColor = raw_ostream::RED;
-static const enum raw_ostream::Colors fatalColor = raw_ostream::RED;
+static constexpr raw_ostream::Colors NoteColor = raw_ostream::CYAN;
+static constexpr raw_ostream::Colors RemarkColor = raw_ostream::BLUE;
+static constexpr raw_ostream::Colors FixitColor = raw_ostream::GREEN;
+static constexpr raw_ostream::Colors CaretColor = raw_ostream::GREEN;
+static constexpr raw_ostream::Colors WarningColor = raw_ostream::MAGENTA;
+static constexpr raw_ostream::Colors templateColor = raw_ostream::CYAN;
+static constexpr raw_ostream::Colors ErrorColor = raw_ostream::RED;
+static constexpr raw_ostream::Colors FatalColor = raw_ostream::RED;
 // Used for changing only the bold attribute.
-static const enum raw_ostream::Colors savedColor =
-  raw_ostream::SAVEDCOLOR;
+static constexpr raw_ostream::Colors SavedColor = raw_ostream::SAVEDCOLOR;
 
 // Magenta is taken for 'warning'. Red is already 'error' and 'cyan'
 // is already taken for 'note'. Green is already used to underline
@@ -99,7 +93,7 @@ static void applyTemplateHighlighting(raw_ostream &OS, StringRef Str,
     else {
       OS.resetColor();
       if (Bold)
-        OS.changeColor(savedColor, true);
+        OS.changeColor(SavedColor, true);
     }
     Normal = !Normal;
   }
@@ -289,47 +283,46 @@ static void genColumnByteMapping(StringRef SourceLine, unsigned TabStop,
 namespace {
 struct SourceColumnMap {
   SourceColumnMap(StringRef SourceLine, unsigned TabStop)
-  : m_SourceLine(SourceLine) {
+      : SourceLine(SourceLine) {
 
-    genColumnByteMapping(SourceLine, TabStop, m_columnToByte, m_byteToColumn);
+    genColumnByteMapping(SourceLine, TabStop, ColumnToByte, ByteToColumn);
 
-    assert(m_byteToColumn.size()==SourceLine.size()+1);
-    assert(0 < m_byteToColumn.size() && 0 < m_columnToByte.size());
-    assert(m_byteToColumn.size() ==
-           static_cast<unsigned>(m_columnToByte.back().V + 1));
-    assert(static_cast<unsigned>(m_byteToColumn.back().V + 1) ==
-           m_columnToByte.size());
+    assert(ByteToColumn.size() == SourceLine.size() + 1);
+    assert(0 < ByteToColumn.size() && 0 < ColumnToByte.size());
+    assert(ByteToColumn.size() ==
+           static_cast<unsigned>(ColumnToByte.back().V + 1));
+    assert(static_cast<unsigned>(ByteToColumn.back().V + 1) ==
+           ColumnToByte.size());
   }
-  Columns columns() const { return m_byteToColumn.back(); }
-  Bytes bytes() const { return m_columnToByte.back(); }
+  Columns columns() const { return ByteToColumn.back(); }
+  Bytes bytes() const { return ColumnToByte.back(); }
 
   /// Map a byte to the column which it is at the start of, or return -1
   /// if it is not at the start of a column (for a UTF-8 trailing byte).
   Columns byteToColumn(Bytes N) const {
-    assert(0 <= N.V && N.V < static_cast<int>(m_byteToColumn.size()));
-    return m_byteToColumn[N.V];
+    assert(0 <= N.V && N.V < static_cast<int>(ByteToColumn.size()));
+    return ByteToColumn[N.V];
   }
 
   /// Map a byte to the first column which contains it.
   Columns byteToContainingColumn(Bytes N) const {
-    assert(0 <= N.V && N.V < static_cast<int>(m_byteToColumn.size()));
-    while (!m_byteToColumn[N.V].isValid())
+    assert(0 <= N.V && N.V < static_cast<int>(ByteToColumn.size()));
+    while (!ByteToColumn[N.V].isValid())
       --N.V;
-    return m_byteToColumn[N.V];
+    return ByteToColumn[N.V];
   }
 
   /// Map a column to the byte which starts the column, or return -1 if
   /// the column the second or subsequent column of an expanded tab or similar
   /// multi-column entity.
   Bytes columnToByte(Columns N) const {
-    assert(0 <= N.V && N.V < static_cast<int>(m_columnToByte.size()));
-    return m_columnToByte[N.V];
+    assert(0 <= N.V && N.V < static_cast<int>(ColumnToByte.size()));
+    return ColumnToByte[N.V];
   }
 
   /// Map from a byte index to the next byte which starts a column.
   Bytes startOfNextColumn(Bytes N) const {
-    assert(0 <= N.V && N.V < static_cast<int>(m_byteToColumn.size() - 1));
-    N = N.next();
+    assert(0 <= N.V && N.V < static_cast<int>(ByteToColumn.size() - 1));
     while (!byteToColumn(N).isValid())
       N = N.next();
     return N;
@@ -337,21 +330,18 @@ struct SourceColumnMap {
 
   /// Map from a byte index to the previous byte which starts a column.
   Bytes startOfPreviousColumn(Bytes N) const {
-    assert(0 < N.V && N.V < static_cast<int>(m_byteToColumn.size()));
-    N = N.prev();
+    assert(0 < N.V && N.V < static_cast<int>(ByteToColumn.size()));
     while (!byteToColumn(N).isValid())
       N = N.prev();
     return N;
   }
 
-  StringRef getSourceLine() const {
-    return m_SourceLine;
-  }
+  StringRef getSourceLine() const { return SourceLine; }
 
 private:
-  StringRef m_SourceLine;
-  SmallVector<Columns, 200> m_byteToColumn;
-  SmallVector<Bytes, 200> m_columnToByte;
+  StringRef SourceLine;
+  SmallVector<Columns, 200> ByteToColumn;
+  SmallVector<Bytes, 200> ColumnToByte;
 };
 } // end anonymous namespace
 
@@ -361,12 +351,12 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
                                           std::string &CaretLine,
                                           std::string &FixItInsertionLine,
                                           Columns NonGutterColumns,
-                                          const SourceColumnMap &map) {
+                                          const SourceColumnMap &Map) {
   Columns CaretColumns = Columns(CaretLine.size());
   Columns FixItColumns =
       Columns(llvm::sys::locale::columnWidth(FixItInsertionLine));
   Columns MaxColumns =
-      std::max({map.columns().V, CaretColumns.V, FixItColumns.V});
+      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;
@@ -415,14 +405,14 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
   // CaretEnd may have been set at the middle of a character
   // If it's not at a character's first column then advance it past the current
   //   character.
-  while (CaretEnd < map.columns() && !map.columnToByte(CaretEnd).isValid())
+  while (CaretEnd < Map.columns() && !Map.columnToByte(CaretEnd).isValid())
     CaretEnd = CaretEnd.next();
 
   assert(
-      (CaretStart > map.columns() || map.columnToByte(CaretStart).isValid()) &&
+      (CaretStart > Map.columns() || Map.columnToByte(CaretStart).isValid()) &&
       "CaretStart must not point to a column in the middle of a source"
       " line character");
-  assert((CaretEnd > map.columns() || map.columnToByte(CaretEnd).isValid()) &&
+  assert((CaretEnd > Map.columns() || Map.columnToByte(CaretEnd).isValid()) &&
          "CaretEnd must not point to a column in the middle of a source line"
          " character");
 
@@ -431,20 +421,19 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
   // number of columns we have, try to grow the slice to encompass
   // more context.
 
-  Bytes SourceStart = map.columnToByte(std::min(CaretStart.V, map.columns().V));
-  Bytes SourceEnd = map.columnToByte(std::min(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 -
-      (map.byteToColumn(SourceEnd) - map.byteToColumn(SourceStart));
+      (Map.byteToColumn(SourceEnd) - Map.byteToColumn(SourceStart));
 
-  char const *front_ellipse = "  ...";
-  char const *front_space   = "     ";
-  char const *back_ellipse = "...";
-  Columns EllipsesColumns =
-      Columns(strlen(front_ellipse) + strlen(back_ellipse));
+  constexpr StringRef FrontEllipse = "  ...";
+  constexpr StringRef FrontSpace = "     ";
+  constexpr StringRef BackEllipse = "...";
+  Columns EllipsesColumns = Columns(FrontEllipse.size() + BackEllipse.size());
 
-  Columns TargetColumns = Columns(NonGutterColumns);
+  Columns TargetColumns = NonGutterColumns;
   // Give us extra room for the ellipses
   //  and any of the caret line that extends past the source
   if (TargetColumns > EllipsesColumns + CaretColumnsOutsideSource)
@@ -454,25 +443,25 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
     bool ExpandedRegion = false;
 
     if (SourceStart > 0) {
-      Bytes NewStart = map.startOfPreviousColumn(SourceStart);
+      Bytes NewStart = Map.startOfPreviousColumn(SourceStart);
 
       // Skip over any whitespace we see here; we're looking for
       // another bit of interesting text.
       // FIXME: Detect non-ASCII whitespace characters too.
       while (NewStart > 0 && isWhitespace(SourceLine[NewStart.V]))
-        NewStart = map.startOfPreviousColumn(NewStart);
+        NewStart = Map.startOfPreviousColumn(NewStart);
 
       // Skip over this bit of "interesting" text.
       while (NewStart > 0) {
-        Bytes Prev = map.startOfPreviousColumn(NewStart);
+        Bytes Prev = Map.startOfPreviousColumn(NewStart);
         if (isWhitespace(SourceLine[Prev.V]))
           break;
         NewStart = Prev;
       }
 
-      assert(map.byteToColumn(NewStart).isValid());
+      assert(Map.byteToColumn(NewStart).isValid());
       Columns NewColumns =
-          map.byteToColumn(SourceEnd) - map.byteToColumn(NewStart);
+          Map.byteToColumn(SourceEnd) - Map.byteToColumn(NewStart);
       if (NewColumns <= TargetColumns) {
         SourceStart = NewStart;
         ExpandedRegion = true;
@@ -480,21 +469,21 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
     }
 
     if (SourceEnd < SourceLine.size()) {
-      Bytes NewEnd = map.startOfNextColumn(SourceEnd);
+      Bytes NewEnd = Map.startOfNextColumn(SourceEnd);
 
       // Skip over any whitespace we see here; we're looking for
       // another bit of interesting text.
       // FIXME: Detect non-ASCII whitespace characters too.
       while (NewEnd < SourceLine.size() && isWhitespace(SourceLine[NewEnd.V]))
-        NewEnd = map.startOfNextColumn(NewEnd);
+        NewEnd = Map.startOfNextColumn(NewEnd);
 
       // Skip over this bit of "interesting" text.
       while (NewEnd < SourceLine.size() && isWhitespace(SourceLine[NewEnd.V]))
-        NewEnd = map.startOfNextColumn(NewEnd);
+        NewEnd = Map.startOfNextColumn(NewEnd);
 
-      assert(map.byteToColumn(NewEnd).isValid());
+      assert(Map.byteToColumn(NewEnd).isValid());
       Columns NewColumns =
-          map.byteToColumn(NewEnd) - map.byteToColumn(SourceStart);
+          Map.byteToColumn(NewEnd) - Map.byteToColumn(SourceStart);
       if (NewColumns <= TargetColumns) {
         SourceEnd = NewEnd;
         ExpandedRegion = true;
@@ -505,8 +494,8 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
       break;
   }
 
-  CaretStart = map.byteToColumn(SourceStart);
-  CaretEnd = map.byteToColumn(SourceEnd) + CaretColumnsOutsideSource;
+  CaretStart = Map.byteToColumn(SourceStart);
+  CaretEnd = Map.byteToColumn(SourceEnd) + CaretColumnsOutsideSource;
 
   // [CaretStart, CaretEnd) is the slice we want. Update the various
   // output lines to show only this slice.
@@ -516,8 +505,8 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
   assert(CaretStart <= CaretEnd);
 
   Columns BackColumnsRemoved =
-      map.byteToColumn(Bytes{static_cast<int>(SourceLine.size())}) -
-      map.byteToColumn(SourceEnd);
+      Map.byteToColumn(Bytes{static_cast<int>(SourceLine.size())}) -
+      Map.byteToColumn(SourceEnd);
   Columns FrontColumnsRemoved = CaretStart;
   Columns ColumnsKept = CaretEnd - CaretStart;
 
@@ -527,19 +516,19 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
 
   // The line needs some truncation, and we'd prefer to keep the front
   //  if possible, so remove the back
-  if (BackColumnsRemoved > Columns(strlen(back_ellipse)))
-    SourceLine.replace(SourceEnd.V, std::string::npos, back_ellipse);
+  if (BackColumnsRemoved > Columns(BackEllipse.size()))
+    SourceLine.replace(SourceEnd.V, std::string::npos, BackEllipse);
 
   // If that's enough then we're done
   if (FrontColumnsRemoved + ColumnsKept <= Columns(NonGutterColumns))
     return;
 
   // Otherwise remove the front as well
-  if (FrontColumnsRemoved > Columns(strlen(front_ellipse))) {
-    SourceLine.replace(0, SourceStart.V, front_ellipse);
-    CaretLine.replace(0, CaretStart.V, front_space);
+  if (FrontColumnsRemoved > Columns(FrontEllipse.size())) {
+    SourceLine.replace(0, SourceStart.V, FrontEllipse);
+    CaretLine.replace(0, CaretStart.V, FrontSpace);
     if (!FixItInsertionLine.empty())
-      FixItInsertionLine.replace(0, CaretStart.V, front_space);
+      FixItInsertionLine.replace(0, CaretStart.V, FrontSpace);
   }
 }
 
@@ -733,11 +722,21 @@ TextDiagnostic::printDiagnosticLevel(raw_ostream &OS,
     switch (Level) {
     case DiagnosticsEngine::Ignored:
       llvm_unreachable("Invalid diagnostic type");
-    case DiagnosticsEngine::Note:    OS.changeColor(noteColor, true); break;
-    case DiagnosticsEngine::Remark:  OS.changeColor(remarkColor, true); break;
-    case DiagnosticsEngine::Warning: OS.changeColor(warningColor, true); break;
-    case DiagnosticsEngine::Error:   OS.changeColor(errorColor, true); break;
-    case DiagnosticsEngine::Fatal:   OS.changeColor(fatalColor, true); break;
+    case DiagnosticsEngine::Note:
+      OS.changeColor(NoteColor, true);
+      break;
+    case DiagnosticsEngine::Remark:
+      OS.changeColor(RemarkColor, true);
+      break;
+    case DiagnosticsEngine::Warning:
+      OS.changeColor(WarningColor, true);
+      break;
+    case DiagnosticsEngine::Error:
+      OS.changeColor(ErrorColor, true);
+      break;
+    case DiagnosticsEngine::Fatal:
+      OS.changeColor(FatalColor, true);
+      break;
     }
   }
 
@@ -765,7 +764,7 @@ void TextDiagnostic::printDiagnosticMessage(raw_ostream &OS,
   if (ShowColors && !IsSupplemental) {
     // Print primary diagnostic messages in bold and without color, to visually
     // indicate the transition from continuation notes and other output.
-    OS.changeColor(savedColor, true);
+    OS.changeColor(SavedColor, true);
     Bold = true;
   }
 
@@ -843,7 +842,7 @@ void TextDiagnostic::emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,
     return;
 
   if (DiagOpts.ShowColors)
-    OS.changeColor(savedColor, true);
+    OS.changeColor(SavedColor, true);
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts.getFormat()) {
@@ -1470,7 +1469,7 @@ void TextDiagnostic::emitSnippetAndCaret(
     if (!CaretLine.empty()) {
       indentForLineNumbers();
       if (DiagOpts.ShowColors)
-        OS.changeColor(caretColor, true);
+        OS.changeColor(CaretColor, true);
       OS << CaretLine << '\n';
       if (DiagOpts.ShowColors)
         OS.resetColor();
@@ -1480,7 +1479,7 @@ void TextDiagnostic::emitSnippetAndCaret(
       indentForLineNumbers();
       if (DiagOpts.ShowColors)
         // Print fixit line in color
-        OS.changeColor(fixitColor, false);
+        OS.changeColor(FixitColor, false);
       if (DiagOpts.ShowSourceRanges)
         OS << ' ';
       OS << FixItInsertionLine << '\n';

And use sizeof() so we can avoid a potential runtime call to strlen.

Also rename map to Map, remove the m_ prefix from member variables and
fix the naming of the existing color variables.
@tbaederr tbaederr merged commit 96feee4 into llvm:main Oct 30, 2025
9 of 10 checks passed
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
Also rename map to Map, remove the m_ prefix from member variables and
fix the naming of the existing color variables.
luciechoi pushed a commit to luciechoi/llvm-project that referenced this pull request Nov 1, 2025
Also rename map to Map, remove the m_ prefix from member variables and
fix the naming of the existing color variables.
DEBADRIBASAK pushed a commit to DEBADRIBASAK/llvm-project that referenced this pull request Nov 3, 2025
Also rename map to Map, remove the m_ prefix from member variables and
fix the naming of the existing color variables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants