Add --incompatible_validate_top_level_header_inclusions

#10047
#9965

With this flag flipped Bazel will also validate header inclusions for top level headers.

RELNOTES: Incompatible flag `--incompatible_validate_top_level_header_inclusions` has been added. See #10047 for details.
PiperOrigin-RevId: 275221690

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java

@@ -487,7 +487,7 @@ private Iterable<Artifact> filterDiscoveredHeaders(
       if (declaredIncludeDirs == null) {
         declaredIncludeDirs = ccCompilationContext.getDeclaredIncludeDirs().toSet();
       }
-      if (!isDeclaredIn(actionExecutionContext, header, declaredIncludeDirs)) {
+      if (!isDeclaredIn(cppConfiguration, actionExecutionContext, header, declaredIncludeDirs)) {
         missing.add(header);
       }
     }
@@ -959,7 +959,7 @@ public void validateInclusions(
       if (declaredIncludeDirs == null) {
         declaredIncludeDirs = Sets.newHashSet(ccCompilationContext.getDeclaredIncludeDirs());
       }
-      if (!isDeclaredIn(actionExecutionContext, input, declaredIncludeDirs)) {
+      if (!isDeclaredIn(cppConfiguration, actionExecutionContext, input, declaredIncludeDirs)) {
         errors.add(input.getExecPath().toString());
       }
     }
@@ -1038,6 +1038,7 @@ void verifyActionIncludePaths(List<PathFragment> systemIncludeDirs)
    * matches.
    */
   private static boolean isDeclaredIn(
+      CppConfiguration cppConfiguration,
       ActionExecutionContext actionExecutionContext,
       Artifact input,
       Set<PathFragment> declaredIncludeDirs) {
@@ -1049,7 +1050,10 @@ private static boolean isDeclaredIn(
     }
     // Need to do dir/package matching: first try a quick exact lookup.
     PathFragment includeDir = input.getRootRelativePath().getParentDirectory();
-    if (includeDir.isEmpty() || declaredIncludeDirs.contains(includeDir)) {
+    if (!cppConfiguration.validateTopLevelHeaderInclusions() && includeDir.isEmpty()) {
+      return true; // Legacy behavior nobody understands anymore.
+    }
+    if (declaredIncludeDirs.contains(includeDir)) {
       return true;  // OK: quick exact match.
     }
     // Not found in the quick lookup: try the wildcards.
@@ -1062,12 +1066,16 @@ private static boolean isDeclaredIn(
     }
     // Still not found: see if it is in a subdir of a declared package.
     Root root = actionExecutionContext.getRoot(input);
+    Path dir = actionExecutionContext.getInputPath(input).getParentDirectory();
+    if (dir.equals(root.asPath())) {
+      return false; // Bad: at the top, give up.
+    }
     // As we walk up along parent paths, we'll need to check whether Bazel build files exist, which
     // would mean that the file is in a sub-package and not a subdir of a declared include
     // directory. Do so lazily to avoid stats when this file doesn't lie beneath any declared
     // include directory.
     List<Path> packagesToCheckForBuildFiles = new ArrayList<>();
-    for (Path dir = actionExecutionContext.getInputPath(input).getParentDirectory(); ; ) {
+    while (true) {
       packagesToCheckForBuildFiles.add(dir);
       dir = dir.getParentDirectory();
       if (dir.equals(root.asPath())) {
@@ -1221,7 +1229,8 @@ public void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp)
         additionalPrunableHeaders,
         ccCompilationContext.getDeclaredIncludeDirs(),
         builtInIncludeDirectories,
-        inputsForInvalidation);
+        inputsForInvalidation,
+        cppConfiguration.validateTopLevelHeaderInclusions());
   }
 
   // Separated into a helper method so that it can be called from CppCompileActionTemplate.
@@ -1238,12 +1247,14 @@ static void computeKey(
       NestedSet<Artifact> prunableHeaders,
       NestedSet<PathFragment> declaredIncludeDirs,
       List<PathFragment> builtInIncludeDirectories,
-      Iterable<Artifact> inputsForInvalidation) {
+      Iterable<Artifact> inputsForInvalidation,
+      boolean validateTopLevelHeaderInclusions) {
     fp.addUUID(actionClassId);
     env.addTo(fp);
     fp.addStringMap(environmentVariables);
     fp.addStringMap(executionInfo);
     fp.addBytes(commandLineKey);
+    fp.addBoolean(validateTopLevelHeaderInclusions);
 
     actionKeyContext.addNestedSetToFingerprint(fp, declaredIncludeSrcs);
     fp.addInt(0); // mark the boundary between input types

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java

@@ -170,7 +170,10 @@ protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp)
         cppCompileActionBuilder.buildPrunableHeaders(),
         cppCompileActionBuilder.getCcCompilationContext().getDeclaredIncludeDirs(),
         cppCompileActionBuilder.getBuiltinIncludeDirectories(),
-        cppCompileActionBuilder.buildInputsForInvalidation());
+        cppCompileActionBuilder.buildInputsForInvalidation(),
+        toolchain
+            .getCppConfigurationEvenThoughItCanBeDifferentThatWhatTargetHas()
+            .validateTopLevelHeaderInclusions());
   }
 
   private boolean shouldCompileHeaders() {

src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java

@@ -698,4 +698,8 @@ public boolean disableNoCopts() {
   public boolean loadCcRulesFromBzl() {
     return cppOptions.loadCcRulesFromBzl;
   }
+
+  public boolean validateTopLevelHeaderInclusions() {
+    return cppOptions.validateTopLevelHeaderInclusions;
+  }
 }

src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java

@@ -764,6 +764,20 @@ public Label getFdoPrefetchHintsLabel() {
               + "(see https://github.com/bazelbuild/bazel/issues/7793 for more information).")
   public boolean requireCtxInConfigureFeatures;
 
+  @Option(
+      name = "incompatible_validate_top_level_header_inclusions",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.INPUT_STRICTNESS,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help =
+          "If true, Bazel will also validate top level directory header inclusions "
+              + "(see https://github.com/bazelbuild/bazel/issues/10047 for more information).")
+  public boolean validateTopLevelHeaderInclusions;
+
   @Option(
       name = "incompatible_remove_legacy_whole_archive",
       defaultValue = "true",
@@ -976,6 +990,7 @@ public FragmentOptions getHost() {
     host.disableStaticCcToolchains = disableStaticCcToolchains;
     host.disableNoCopts = disableNoCopts;
     host.loadCcRulesFromBzl = loadCcRulesFromBzl;
+    host.validateTopLevelHeaderInclusions = validateTopLevelHeaderInclusions;
 
     // Save host options for further use.
     host.hostCoptList = hostCoptList;

src/test/shell/bazel/cc_integration_test.sh

@@ -599,4 +599,38 @@ EOF
   bazel-bin/"$pkg"/g | grep a1a2bcddcb2a1a || fail "output is incorrect"
 }
 
+function test_incompatible_validate_top_level_header_inclusions() {
+  local workspace="${FUNCNAME[0]}"
+  mkdir -p "${workspace}"
+
+  touch "${workspace}/WORKSPACE"
+  cat >> "${workspace}/BUILD" << EOF
+cc_library(
+    name = "foo",
+    srcs = ["foo.cc"],
+)
+EOF
+  cat >> "${workspace}/foo.cc" << EOF
+#include "top_level.h"
+
+int foo() {
+  return bar();
+}
+EOF
+cat >> "${workspace}/top_level.h" << EOF
+inline int bar() { return 42; }
+EOF
+
+  cd "${workspace}"
+  bazel build --noincompatible_validate_top_level_header_inclusions \
+  --spawn_strategy=standalone \
+    //:foo  &>"$TEST_log" || fail "Build failed but should have succeeded"
+
+  bazel build --incompatible_validate_top_level_header_inclusions \
+  --spawn_strategy=standalone \
+    //:foo  &>"$TEST_log" && fail "Build succeeded but should have failed"
+  expect_log "this rule is missing dependency declarations for the "\
+    "following files included by 'foo.cc'"
+}
+
 run_suite "cc_integration_test"