[clang-scan-deps] do not skip empty #if/#elif in the minimize to avoid missing `__has_include` dependencies

[arphaman]

Summary

This patch makes the minimizer more conservative to avoid missing dependency files that are brought in by __has_include PP expressions that occur in a condition of an #if/#elif that was previously skipped. The __has_include PP expressions can be used in an #if/#elif either directly, or through macro expansion, so we can't detect them at the time of minimization.

It'll be possible to skip certain #if/#elif in the future by doing more minimizer optimizations (e.g. skip #if 0). Furthermore, @Bigcheese is working on clarifying the allowed usage of __has_include in the standard, as it seems that this kind of usage should actually be disallowed. This will hopefully allow us skip these #if/#elif again in the future.

[dexonsmith]

LGTM, although I have a suggested change for one of the comments (and I'm still sad about this).

clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

766–769

I'd prefer to structure this comment differently, something like:

// If "#ifdef" is empty, strip it and skip the "#endif".
//
// TODO: Once/if Clang starts disallowing __has_include in macro expansions,
// we can skip empty `#if` and `#elif` blocks as well after scanning for a
// literal __has_include in the condition.  Even without that rule we could
// drop the tokens if we scan for identifiers in the condition and find none.

This rephrases it as a potential future optimization instead of one that has to be skipped, which I think makes it easier to reason about.

clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

bool Minimizer::lexPragma(const char *&First, const char *const End) {
   return false;
 }
 
 bool Minimizer::lexEndif(const char *&First, const char *const End) {
   // Strip out "#else" if it's empty.
   if (top() == pp_else)
     popToken();
 
-  // Strip out "#elif" if they're empty.
-  while (top() == pp_elif)
-    popToken();
-
-  // If "#if" is empty, strip it and skip the "#endif".
-  if (top() == pp_if || top() == pp_ifdef || top() == pp_ifndef) {
+  // If "#ifdef" is empty, strip it and skip the "#endif".
+  // Avoid stripping "#if" / "#elif" as they could contain a "__has_include"
+  // (either directly, or through a macro expansion) without an "#include"
+  // inside the "#if/elif", which could affect the dependency output.
+  if (top() == pp_ifdef || top() == pp_ifndef) {
     popToken();
     skipLine(First, End);
     return false;
   }
 
   return lexDefault(pp_endif, "endif", First, End);
 }
 

clang/test/ClangScanDeps/Inputs/has_include_if_elif.json

+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/has_include_if_elif2.cpp -IInputs",
+  "file": "DIR/has_include_if_elif2.cpp"
+}
+]

clang/test/ClangScanDeps/has_include_if_elif.cpp

+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/has_include_if_elif2.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header2.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header3.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header4.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/has_include_if_elif.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | \
+// RUN:   FileCheck %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess | \
+// RUN:   FileCheck %s
+
+#if __has_include("header.h")
+#endif
+
+#if 0
+#elif __has_include("header2.h")
+#endif
+
+#define H3 __has_include("header3.h")
+#if H3
+#endif
+
+#define H4 __has_include("header4.h")
+
+#if 0
+#elif H4
+#endif
+
+// CHECK: has_include_if_elif2.cpp
+// CHECK-NEXT: Inputs{{/|\\}}header.h
+// CHECK-NEXT: Inputs{{/|\\}}header2.h
+// CHECK-NEXT: Inputs{{/|\\}}header3.h
+// CHECK-NEXT: Inputs{{/|\\}}header4.h

clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

EXPECT_STREQ("#ifdef A\n"
                "#endif\n",
                Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, EmptyIfdef) {
   SmallVector<char, 128> Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#ifdef A\n"
+                                                    "void skip();\n"
                                                     "#elif B\n"
                                                     "#elif C\n"
                                                     "#else D\n"
                                                     "#endif\n",
                                                     Out));
-  EXPECT_STREQ("", Out.data());
+  EXPECT_STREQ("#ifdef A\n"
+               "#elif B\n"
+               "#elif C\n"
+               "#endif\n",
+               Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, Pragma) {
   SmallVector<char, 128> Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#pragma A\n", Out));
   EXPECT_STREQ("", Out.data());
 
for (auto Source : {
        }) {
     ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
     EXPECT_STREQ("#include <t.h>\n", Out.data());
   }
 
   for (auto Source : {
            "#warning \\\n#include <t.h>\n",
            "#error \\\n#include <t.h>\n",
+       }) {
+    ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+    EXPECT_STREQ("", Out.data());
+  }
+
+  for (auto Source : {
            "#if MACRO\n#warning '\n#endif\n",
            "#if MACRO\n#warning \"\n#endif\n",
            "#if MACRO\n#warning /*\n#endif\n",
            "#if MACRO\n#error '\n#endif\n",
            "#if MACRO\n#error \"\n#endif\n",
            "#if MACRO\n#error /*\n#endif\n",
        }) {
     ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
-    EXPECT_STREQ("", Out.data());
+    EXPECT_STREQ("#if MACRO\n#endif\n", Out.data());
   }
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, CharacterLiteral) {
   SmallVector<char, 128> Out;
 
   StringRef Source = R"(
 #include <bob>
TEST(MinimizeSourceToDependencyDirectivesTest, CharacterLiteralPrefixL) {
 
   StringRef Source = R"(L'P'
 #if DEBUG
 // '
 #endif
 #include <test.h>
 )";
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
-  EXPECT_STREQ("#include <test.h>\n", Out.data());
+  EXPECT_STREQ("#if DEBUG\n#endif\n#include <test.h>\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, CharacterLiteralPrefixU) {
   SmallVector<char, 128> Out;
 
   StringRef Source = R"(int x = U'P';
 #include <test.h>
 // '