[Lldb-commits] [lldb] 8715ffd - [lldb] Fix that trailing backslashes in source lines break the Clang highlighter

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-11-14T11:11:20+01:00
New Revision: 8715ffdf1aafbfca7c3d7f1622fe586243f31df1

URL: https://github.com/llvm/llvm-project/commit/8715ffdf1aafbfca7c3d7f1622fe586243f31df1
DIFF: https://github.com/llvm/llvm-project/commit/8715ffdf1aafbfca7c3d7f1622fe586243f31df1.diff

LOG: [lldb] Fix that trailing backslashes in source lines break the Clang highlighter

Summary:
Clang's raw Lexer doesn't produce any tokens for trailing backslashes in a line. This doesn't work with
LLDB's Clang highlighter which builds the source code to display from the list of tokens the Lexer returns.
This causes that lines with trailing backslashes are lacking the backslash and the following newline when
rendering source code in LLDB.

This patch removes the trailing newline from the current line we are highlighting. This way Clang doesn't
drop the backslash token and we just restore the newline after tokenising.

Fixes rdar://57091487

Reviewers: JDevlieghere, labath

Reviewed By: JDevlieghere, labath

Subscribers: labath, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D70177

Added: 
    

Modified: 
    lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
    lldb/unittests/Language/Highlighting/HighlighterTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp b/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
index a9a1b44731f2..3e77b1646739 100644
--- a/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
+++ b/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
@@ -139,6 +139,22 @@ void ClangHighlighter::Highlight(const HighlightStyle &options,
   FileManager file_mgr(file_opts,
                        FileSystem::Instance().GetVirtualFileSystem());
 
+  // The line might end in a backslash which would cause Clang to drop the
+  // backslash and the terminating new line. This makes sense when parsing C++,
+  // but when highlighting we care about preserving the backslash/newline. To
+  // not lose this information we remove the new line here so that Clang knows
+  // this is just a single line we are highlighting. We add back the newline
+  // after tokenizing.
+  llvm::StringRef line_ending = "";
+  // There are a few legal line endings Clang recognizes and we need to
+  // temporarily remove from the string.
+  if (line.consume_back("\r\n"))
+    line_ending = "\r\n";
+  else if (line.consume_back("\n"))
+    line_ending = "\n";
+  else if (line.consume_back("\r"))
+    line_ending = "\r";
+
   unsigned line_number = previous_lines.count('\n') + 1U;
 
   // Let's build the actual source code Clang needs and setup some utility
@@ -227,6 +243,9 @@ void ClangHighlighter::Highlight(const HighlightStyle &options,
     color.Apply(result, to_print);
   }
 
+  // Add the line ending we trimmed before tokenizing.
+  result << line_ending;
+
   // If we went over the whole file but couldn't find our own file, then
   // somehow our setup was wrong. When we're in release mode we just give the
   // user the normal line and pretend we don't know how to highlight it. In

diff  --git a/lldb/unittests/Language/Highlighting/HighlighterTest.cpp b/lldb/unittests/Language/Highlighting/HighlighterTest.cpp
index 71457ed61892..ebcc6122245f 100644
--- a/lldb/unittests/Language/Highlighting/HighlighterTest.cpp
+++ b/lldb/unittests/Language/Highlighting/HighlighterTest.cpp
@@ -205,6 +205,44 @@ TEST_F(HighlighterTest, ClangPPDirectives) {
             highlightC("#include \"foo\" //c", s));
 }
 
+TEST_F(HighlighterTest, ClangPreserveNewLine) {
+  HighlightStyle s;
+  s.comment.Set("<cc>", "</cc>");
+
+  EXPECT_EQ("<cc>//</cc>\n", highlightC("//\n", s));
+}
+
+TEST_F(HighlighterTest, ClangTrailingBackslashBeforeNewline) {
+  HighlightStyle s;
+
+  EXPECT_EQ("\\\n", highlightC("\\\n", s));
+  EXPECT_EQ("\\\r\n", highlightC("\\\r\n", s));
+
+  EXPECT_EQ("#define a \\\n", highlightC("#define a \\\n", s));
+  EXPECT_EQ("#define a \\\r\n", highlightC("#define a \\\r\n", s));
+  EXPECT_EQ("#define a \\\r", highlightC("#define a \\\r", s));
+}
+
+TEST_F(HighlighterTest, ClangTrailingBackslashWithWhitespace) {
+  HighlightStyle s;
+
+  EXPECT_EQ("\\  \n", highlightC("\\  \n", s));
+  EXPECT_EQ("\\ \t\n", highlightC("\\ \t\n", s));
+  EXPECT_EQ("\\ \n", highlightC("\\ \n", s));
+  EXPECT_EQ("\\\t\n", highlightC("\\\t\n", s));
+
+  EXPECT_EQ("#define a \\  \n", highlightC("#define a \\  \n", s));
+  EXPECT_EQ("#define a \\ \t\n", highlightC("#define a \\ \t\n", s));
+  EXPECT_EQ("#define a \\ \n", highlightC("#define a \\ \n", s));
+  EXPECT_EQ("#define a \\\t\n", highlightC("#define a \\\t\n", s));
+}
+
+TEST_F(HighlighterTest, ClangTrailingBackslashMissingNewLine) {
+  HighlightStyle s;
+  EXPECT_EQ("\\", highlightC("\\", s));
+  EXPECT_EQ("#define a\\", highlightC("#define a\\", s));
+}
+
 TEST_F(HighlighterTest, ClangComments) {
   HighlightStyle s;
   s.comment.Set("<cc>", "</cc>");