Fix for stringized function-macro args continued across lines

[kousikk]

Summary

In case of certain #define'd macros, there's a space just before line continuation
that the minimized-source lexer was missing to include, resulting in invalid stringize.
For example:

#define foo(x) x+7
#define bar(a,b) \
    #x \
    foo(b)

should result in "#x foo(b)", but actually results in "#xfoo(b)" (which is invalid).

Fixing it by keeping last but one space. I am confident that keeping the space is the right fix,
although I'm not sure if I've done it in the right way :)

[kousikk]

Ping on the review for this :)

[arphaman]

LGTM, thanks!

[kousikk]

Thanks! I will need you to merge this one too!

[arphaman]

In D67635#1675606, @kousikk wrote:

Thanks! I will need you to merge this one too!

Sure, I will commit it today.

I think you should obtain commit access for your future patches/commits. You can follow the instructions here: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

[kousikk]

I think you should obtain commit access for your future patches/commits. You can follow the instructions here: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

Thanks, I've done that!

clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

for (;;) {
 
     First += Len;
     // Keep skipping lines...
   }
 }
 
 static const char *reverseOverSpaces(const char *First, const char *Last) {
   assert(First <= Last);
-  while (First != Last && isHorizontalWhitespace(Last[-1]))
+  const char *PrevLast = Last;
+  while (First != Last && isHorizontalWhitespace(Last[-1])) {
+    PrevLast = Last;
     --Last;
-  return Last;
+  }
+  return PrevLast;
 }
 
 static void skipLineComment(const char *&First, const char *const End) {
   assert(First[0] == '/' && First[1] == '/');
   First += 2;
   skipToNewlineRaw(First, End);
 }
 

clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

TEST(MinimizeSourceToDependencyDirectivesTest, DefineInvalidMacroArguments) {
   EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineHorizontalWhitespace) {
   SmallVector<char, 128> Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
       "#define MACRO(\t)\tcon \t tent\t", Out));
-  EXPECT_STREQ("#define MACRO() con \t tent\n", Out.data());
+  EXPECT_STREQ("#define MACRO() con \t tent\t\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
       "#define MACRO(\f)\fcon \f tent\f", Out));
-  EXPECT_STREQ("#define MACRO() con \f tent\n", Out.data());
+  EXPECT_STREQ("#define MACRO() con \f tent\f\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
       "#define MACRO(\v)\vcon \v tent\v", Out));
-  EXPECT_STREQ("#define MACRO() con \v tent\n", Out.data());
+  EXPECT_STREQ("#define MACRO() con \v tent\v\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
       "#define MACRO \t\v\f\v\t con\f\t\vtent\v\f \v", Out));
-  EXPECT_STREQ("#define MACRO con\f\t\vtent\n", Out.data());
+  EXPECT_STREQ("#define MACRO con\f\t\vtent\v\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineMultilineArgs) {
   SmallVector<char, 128> Out;
 
   ASSERT_FALSE(
       minimizeSourceToDependencyDirectives("#define MACRO(a        \\\n"
                                            "              )",
                                            Out));
   EXPECT_STREQ("#define MACRO(a)\n", Out.data());
 
   ASSERT_FALSE(
       minimizeSourceToDependencyDirectives("#define MACRO(a,       \\\n"
                                            "              b)       \\\n"
                                            "        call((a),      \\\n"
                                            "             (b))",
                                            Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest,
      DefineMultilineArgsCarriageReturn) {
   SmallVector<char, 128> Out;
 
   ASSERT_FALSE(
       minimizeSourceToDependencyDirectives("#define MACRO(a,       \\\r"
                                            "              b)       \\\r"
                                            "        call((a),      \\\r"
                                            "             (b))",
                                            Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest,
+     DefineMultilineArgsStringize) {
+  SmallVector<char, 128> Out;
+
+  ASSERT_FALSE(
+      minimizeSourceToDependencyDirectives("#define MACRO(a,b) \\\n"
+                                           "                #a \\\n"
+                                           "                #b",
+                                           Out));
+  EXPECT_STREQ("#define MACRO(a,b) #a #b\n", Out.data());
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest,
      DefineMultilineArgsCarriageReturnNewline) {
   SmallVector<char, 128> Out;
 
   ASSERT_FALSE(
       minimizeSourceToDependencyDirectives("#define MACRO(a,       \\\r\n"
                                            "              b)       \\\r\n"
                                            "        call((a),      \\\r\n"
                                            "             (b))",
                                            Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest,
      DefineMultilineArgsNewlineCarriageReturn) {
   SmallVector<char, 128> Out;
 
   ASSERT_FALSE(
       minimizeSourceToDependencyDirectives("#define MACRO(a,       \\\n\r"
                                            "              b)       \\\n\r"
                                            "        call((a),      \\\n\r"
                                            "             (b))",
                                            Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineNumber) {
   SmallVector<char, 128> Out;
 
   ASSERT_TRUE(minimizeSourceToDependencyDirectives("#define 0\n", Out));
 }