[clang-scan-deps] Dependency directives source minimizer: single quotes are not digit separators after a valid character literal prefix

[arphaman]

Summary

The single quote character can act as a c++ digit separator. However, the minimizer shouldn't treat it as such when it's actually following a valid character literal prefix, like L, U, u, or u8.

[dexonsmith]

LGTM, but I have a suggestion inline for another approach.

clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

265–272

I wonder if this would be easier to identify walking forward from Start rather than working backwards from Cur.

[Bigcheese]

clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

270

Are we sure at this point that it's always safe to jump back 2?

[arphaman]

clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

265–272

Yes, that's a possible option. We can scan scanning from start, until we reach non-whitespace right before Cur, and then identify the token.

In such ambiguous cases it might be a good idea to raw lex the line using the Lexer from Start to End. Then we'll match the behavior of Lexer when there's an actual error as well. I'll see if I can setup a fallback like this in a follow-up patch.

[arphaman]

clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

270

It should be, because otherwise Start would've been equals to Cur - 1 which we check for right before the dereference.

[Bigcheese]

clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

270

Oh, obviously. I missed that check.

clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

static bool isQuoteCppDigitSeparator(const char *const Start,
                                      const char *const Cur,
                                      const char *const End) {
   assert(*Cur == '\'' && "expected quotation character");
   // skipLine called in places where we don't expect a valid number
   // body before `start` on the same line, so always return false at the start.
   if (Start == Cur)
     return false;
   // The previous character must be a valid PP number character.
-  if (!isPreprocessingNumberBody(*(Cur - 1)))
+  // Make sure that the L, u, U, u8 prefixes don't get marked as a
+  // separator though.
+  char Prev = *(Cur - 1);
+  if (Prev == 'L' || Prev == 'U' || Prev == 'u')
+    return false;
+  if (Prev == '8' && (Cur - 1 != Start) && *(Cur - 2) == 'u')
+    return false;
+  if (!isPreprocessingNumberBody(Prev))
     return false;
   // The next character should be a valid identifier body character.
   return (Cur + 1) < End && isIdentifierBody(*(Cur + 1));
 }
 
 static void skipLine(const char *&First, const char *const End) {
   do {
     assert(First <= End);

clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

 int b = 0xfa'af'fa;
 int c = 12 ' ';
 #include <foo>
 )";
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#include <bob>\n#include <foo>\n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest, CharacterLiteralPrefixL) {
+  SmallVector<char, 128> Out;
+
+  StringRef Source = R"(L'P'
+#if DEBUG
+// '
+#endif
+#include <test.h>
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#include <test.h>\n", Out.data());
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest, CharacterLiteralPrefixU) {
+  SmallVector<char, 128> Out;
+
+  StringRef Source = R"(int x = U'P';
+#include <test.h>
+// '
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#include <test.h>\n", Out.data());
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest, CharacterLiteralPrefixu) {
+  SmallVector<char, 128> Out;
+
+  StringRef Source = R"(int x = u'b';
+int y = u8'a';
+int z = 128'78;
+#include <test.h>
+// '
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#include <test.h>\n", Out.data());
+}
+
 } // end anonymous namespace