[clang-scan-deps] Improve string/character literal skipping

hyp · hyp · commit a13f0da1d0bc · 2019-10-08T22:42:44.000Z
The existing string/character literal skipping code in the dependency directives source minimizer has two issues: - It doesn't stop the scanning when a newline is reached before the terminating character, unlike the lexer which considers the token to be done (even if it's invalid) at the end of the line. - It doesn't support whitespace between '\' and the newline when looking if the '\' is used as a line continuation character. This commit fixes both issues. Differential Revision: https://reviews.llvm.org/D68436 llvm-svn: 374127
diff --git a/clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp b/clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -185,17 +185,6 @@ static void skipRawString(const char *&First, const char *const End) {
   }
 }
 
-static void skipString(const char *&First, const char *const End) {
-  assert(*First == '\'' || *First == '"' || *First == '<');
-  const char Terminator = *First == '<' ? '>' : *First;
-  for (++First; First != End && *First != Terminator; ++First)
-    if (*First == '\\')
-      if (++First == End)
-        return;
-  if (First != End)
-    ++First; // Finish off the string.
-}
-
 // Returns the length of EOL, either 0 (no end-of-line), 1 (\n) or 2 (\r\n)
 static unsigned isEOL(const char *First, const char *const End) {
   if (First == End)
@@ -206,6 +195,35 @@ static unsigned isEOL(const char *First, const char *const End) {
   return !!isVerticalWhitespace(First[0]);
 }
 
+static void skipString(const char *&First, const char *const End) {
+  assert(*First == '\'' || *First == '"' || *First == '<');
+  const char Terminator = *First == '<' ? '>' : *First;
+  for (++First; First != End && *First != Terminator; ++First) {
+    // String and character literals don't extend past the end of the line.
+    if (isVerticalWhitespace(*First))
+      return;
+    if (*First != '\\')
+      continue;
+    // Skip past backslash to the next character. This ensures that the
+    // character right after it is skipped as well, which matters if it's
+    // the terminator.
+    if (++First == End)
+      return;
+    if (!isWhitespace(*First))
+      continue;
+    // Whitespace after the backslash might indicate a line continuation.
+    const char *FirstAfterBackslashPastSpace = First;
+    skipOverSpaces(FirstAfterBackslashPastSpace, End);
+    if (unsigned NLSize = isEOL(FirstAfterBackslashPastSpace, End)) {
+      // Advance the character pointer to the next line for the next
+      // iteration.
+      First = FirstAfterBackslashPastSpace + NLSize - 1;
+    }
+  }
+  if (First != End)
+    ++First; // Finish off the string.
+}
+
 // Returns the length of the skipped newline
 static unsigned skipNewline(const char *&First, const char *End) {
   if (First == End)
diff --git a/clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp b/clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -594,6 +594,50 @@ TEST(MinimizeSourceToDependencyDirectivesTest, PragmaOnce) {
   EXPECT_STREQ("#pragma once\n#include <test.h>\n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest,
+     SkipLineStringCharLiteralsUntilNewline) {
+  SmallVector<char, 128> Out;
+
+  StringRef Source = R"(#if NEVER_ENABLED
+    #define why(fmt, ...) #error don't try me
+    #endif
+
+    void foo();
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ(
+      "#if NEVER_ENABLED\n#define why(fmt,...) #error don't try me\n#endif\n",
+      Out.data());
+
+  Source = R"(#if NEVER_ENABLED
+      #define why(fmt, ...) "quote dropped
+      #endif
+
+      void foo();
+  )";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ(
+      "#if NEVER_ENABLED\n#define why(fmt,...) \"quote dropped\n#endif\n",
+      Out.data());
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest,
+     SupportWhitespaceBeforeLineContinuationInStringSkipping) {
+  SmallVector<char, 128> Out;
+
+  StringRef Source = "#define X '\\ \t\nx'\nvoid foo() {}";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define X '\\ \t\nx'\n", Out.data());
+
+  Source = "#define X \"\\ \r\nx\"\nvoid foo() {}";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define X \"\\ \r\nx\"\n", Out.data());
+
+  Source = "#define X \"\\ \r\nx\n#include <x>\n";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define X \"\\ \r\nx\n#include <x>\n", Out.data());
+}
+
 TEST(MinimizeSourceToDependencyDirectivesTest, CxxModules) {
   SmallVector<char, 128> Out;
   SmallVector<Token, 4> Tokens;
