Remove "warn" setting for hdrs_check. This has not proven useful.

For now, implicitly convert "warn" to "loose".

RELNOTES: None.
PiperOrigin-RevId: 205652060

5 files changed, 21 insertions, 88 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index f09645c006..b6c55be56c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -583,7 +583,7 @@ public final class CcCommon {
 
   /**
    * Determines a list of loose include directories that are only allowed to be referenced when
-   * headers checking is {@link HeadersCheckingMode#LOOSE} or {@link HeadersCheckingMode#WARN}.
+   * headers checking is {@link HeadersCheckingMode#LOOSE}.
    */
   Set<PathFragment> getLooseIncludeDirs() {
     ImmutableSet.Builder<PathFragment> result = ImmutableSet.builder();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
index a9f8b406f3..160218c0ff 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
@@ -57,7 +57,6 @@ public final class CcCompilationContext implements CcCompilationContextApi {
   private final CommandLineCcCompilationContext commandLineCcCompilationContext;
 
   private final NestedSet<PathFragment> declaredIncludeDirs;
-  private final NestedSet<PathFragment> declaredIncludeWarnDirs;
   private final NestedSet<Artifact> declaredIncludeSrcs;
 
   /** Module maps from direct dependencies. */
@@ -85,7 +84,6 @@ public final class CcCompilationContext implements CcCompilationContextApi {
       CommandLineCcCompilationContext commandLineCcCompilationContext,
       ImmutableSet<Artifact> compilationPrerequisites,
       NestedSet<PathFragment> declaredIncludeDirs,
-      NestedSet<PathFragment> declaredIncludeWarnDirs,
       NestedSet<Artifact> declaredIncludeSrcs,
       NestedSet<Artifact> nonCodeInputs,
       HeaderInfo headerInfo,
@@ -99,7 +97,6 @@ public final class CcCompilationContext implements CcCompilationContextApi {
     Preconditions.checkNotNull(commandLineCcCompilationContext);
     this.commandLineCcCompilationContext = commandLineCcCompilationContext;
     this.declaredIncludeDirs = declaredIncludeDirs;
-    this.declaredIncludeWarnDirs = declaredIncludeWarnDirs;
     this.declaredIncludeSrcs = declaredIncludeSrcs;
     this.directModuleMaps = directModuleMaps;
     this.headerInfo = headerInfo;
@@ -178,14 +175,6 @@ public final class CcCompilationContext implements CcCompilationContextApi {
   }
 
   /**
-   * Returns the immutable set of include directories, relative to a "-I" or "-iquote" directory",
-   * from which inclusion will produce a warning (possibly empty but never null).
-   */
-  public NestedSet<PathFragment> getDeclaredIncludeWarnDirs() {
-    return declaredIncludeWarnDirs;
-  }
-
-  /**
    * Returns the immutable set of headers that have been declared in the {@code srcs} or {@code
    * hdrs} attribute (possibly empty but never null).
    */
@@ -299,8 +288,7 @@ public final class CcCompilationContext implements CcCompilationContextApi {
 
   /**
    * Returns a {@code CcCompilationContext} that is based on a given {@code CcCompilationContext}
-   * but returns empty sets for {@link #getDeclaredIncludeDirs()} and {@link
-   * #getDeclaredIncludeWarnDirs()}.
+   * but returns empty sets for {@link #getDeclaredIncludeDirs()}.
    */
   public static CcCompilationContext disallowUndeclaredHeaders(
       CcCompilationContext ccCompilationContext) {
@@ -308,7 +296,6 @@ public final class CcCompilationContext implements CcCompilationContextApi {
         ccCompilationContext.commandLineCcCompilationContext,
         ccCompilationContext.compilationPrerequisites,
         NestedSetBuilder.emptySet(Order.STABLE_ORDER),
-        NestedSetBuilder.emptySet(Order.STABLE_ORDER),
         ccCompilationContext.declaredIncludeSrcs,
         ccCompilationContext.nonCodeInputs,
         ccCompilationContext.headerInfo,
@@ -365,8 +352,6 @@ public final class CcCompilationContext implements CcCompilationContextApi {
     private final Set<PathFragment> systemIncludeDirs = new LinkedHashSet<>();
     private final NestedSetBuilder<PathFragment> declaredIncludeDirs =
         NestedSetBuilder.stableOrder();
-    private final NestedSetBuilder<PathFragment> declaredIncludeWarnDirs =
-        NestedSetBuilder.stableOrder();
     private final NestedSetBuilder<Artifact> declaredIncludeSrcs =
         NestedSetBuilder.stableOrder();
     private final NestedSetBuilder<Artifact> nonCodeInputs = NestedSetBuilder.stableOrder();
@@ -420,7 +405,6 @@ public final class CcCompilationContext implements CcCompilationContextApi {
       quoteIncludeDirs.addAll(otherCcCompilationContext.getQuoteIncludeDirs());
       systemIncludeDirs.addAll(otherCcCompilationContext.getSystemIncludeDirs());
       declaredIncludeDirs.addTransitive(otherCcCompilationContext.getDeclaredIncludeDirs());
-      declaredIncludeWarnDirs.addTransitive(otherCcCompilationContext.getDeclaredIncludeWarnDirs());
       declaredIncludeSrcs.addTransitive(otherCcCompilationContext.getDeclaredIncludeSrcs());
       transitiveHeaderInfo.addTransitive(otherCcCompilationContext.transitiveHeaderInfos);
       transitiveModules.addTransitive(otherCcCompilationContext.transitiveModules);
@@ -505,25 +489,13 @@ public final class CcCompilationContext implements CcCompilationContextApi {
       return this;
     }
 
-    /**
-     * Add a single declared include dir, relative to a "-I" or "-iquote"
-     * directory".
-     */
+    /** Add a single declared include dir, relative to a "-I" or "-iquote" directory". */
     public Builder addDeclaredIncludeDir(PathFragment dir) {
       declaredIncludeDirs.add(dir);
       return this;
     }
 
     /**
-     * Add a single declared include directory, relative to a "-I" or "-iquote"
-     * directory", from which inclusion will produce a warning.
-     */
-    public Builder addDeclaredIncludeWarnDir(PathFragment dir) {
-      declaredIncludeWarnDirs.add(dir);
-      return this;
-    }
-
-    /**
      * Adds a header that has been declared in the {@code src} or {@code headers attribute}. The
      * header will also be added to the compilation prerequisites.
      *
@@ -661,7 +633,6 @@ public final class CcCompilationContext implements CcCompilationContextApi {
               ? ImmutableSet.copyOf(compilationPrerequisites)
               : ImmutableSet.of(prerequisiteStampFile),
           declaredIncludeDirs.build(),
-          declaredIncludeWarnDirs.build(),
           declaredIncludeSrcs.build(),
           nonCodeInputs.build(),
           headerInfo,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
index 09ea9f6596..1aef25f201 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -587,8 +587,7 @@ public final class CcCompilationHelper {
 
   /**
    * Sets the given directories to by loose include directories that are only allowed to be
-   * referenced when headers checking is {@link HeadersCheckingMode#LOOSE} or {@link
-   * HeadersCheckingMode#WARN}.
+   * referenced when headers checking is {@link HeadersCheckingMode#LOOSE}.
    */
   private void setLooseIncludeDirs(Set<PathFragment> looseIncludeDirs) {
     this.looseIncludeDirs = looseIncludeDirs;
@@ -983,13 +982,7 @@ public final class CcCompilationHelper {
 
     // Add this package's dir to declaredIncludeDirs, & this rule's headers to declaredIncludeSrcs
     // Note: no include dir for STRICT mode.
-    if (headersCheckingMode == HeadersCheckingMode.WARN) {
-      ccCompilationContextBuilder.addDeclaredIncludeWarnDir(
-          ruleContext.getLabel().getPackageFragment());
-      for (PathFragment looseIncludeDir : looseIncludeDirs) {
-        ccCompilationContextBuilder.addDeclaredIncludeWarnDir(looseIncludeDir);
-      }
-    } else if (headersCheckingMode == HeadersCheckingMode.LOOSE) {
+    if (headersCheckingMode == HeadersCheckingMode.LOOSE) {
       ccCompilationContextBuilder.addDeclaredIncludeDir(
           ruleContext.getLabel().getPackageFragment());
       for (PathFragment looseIncludeDir : looseIncludeDirs) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 8683482f93..bea097192c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -45,13 +45,11 @@ import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.actions.extra.CppCompileInfo;
 import com.google.devtools.build.lib.actions.extra.EnvironmentVariable;
 import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
-import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.CollectionUtils;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -89,8 +87,7 @@ public class CppCompileAction extends AbstractAction
 
   private static final PathFragment BUILD_PATH_FRAGMENT = PathFragment.create("BUILD");
 
-  private static final int VALIDATION_DEBUG = 0;  // 0==none, 1==warns/errors, 2==all
-  private static final boolean VALIDATION_DEBUG_WARN = VALIDATION_DEBUG >= 1;
+  private static final boolean VALIDATION_DEBUG_WARN = false;
 
   protected final Artifact outputFile;
   private final Artifact sourceFile;
@@ -430,9 +427,7 @@ public class CppCompileAction extends AbstractAction
         continue;
       }
       if (declaredIncludeDirs == null) {
-        declaredIncludeDirs =
-            new HashSet<>(ccCompilationContext.getDeclaredIncludeWarnDirs().toCollection());
-        declaredIncludeDirs.addAll(ccCompilationContext.getDeclaredIncludeDirs().toCollection());
+        declaredIncludeDirs = ccCompilationContext.getDeclaredIncludeDirs().toSet();
       }
       if (isDeclaredIn(actionExecutionContext, header, declaredIncludeDirs)) {
         result.add(header);
@@ -736,7 +731,6 @@ public class CppCompileAction extends AbstractAction
       ActionExecutionContext actionExecutionContext, Iterable<Artifact> inputsForValidation)
       throws ActionExecutionException {
     IncludeProblems errors = new IncludeProblems();
-    IncludeProblems warnings = new IncludeProblems();
     Set<Artifact> allowedIncludes = new HashSet<>();
     for (Artifact input :
         Iterables.concat(
@@ -757,7 +751,6 @@ public class CppCompileAction extends AbstractAction
     // Copy the nested sets to hash sets for fast contains checking, but do so lazily.
     // Avoid immutable sets here to limit memory churn.
     Set<PathFragment> declaredIncludeDirs = null;
-    Set<PathFragment> warnIncludeDirs = null;
     for (Artifact input : inputsForValidation) {
       // Only declared modules are added to an action and so they are always valid.
       if (input.isFileType(CppFileTypes.CPP_MODULE)) {
@@ -774,24 +767,14 @@ public class CppCompileAction extends AbstractAction
         declaredIncludeDirs = Sets.newHashSet(ccCompilationContext.getDeclaredIncludeDirs());
       }
       if (!isDeclaredIn(actionExecutionContext, input, declaredIncludeDirs)) {
-        if (warnIncludeDirs == null) {
-          warnIncludeDirs = Sets.newHashSet(ccCompilationContext.getDeclaredIncludeWarnDirs());
-        }
-        // This call can never match the declared include sources (they would be matched above).
-        if (isDeclaredIn(actionExecutionContext, input, warnIncludeDirs)) {
-          warnings.add(input.getExecPath().toString());
-        } else {
-          errors.add(input.getExecPath().toString());
-        }
+        errors.add(input.getExecPath().toString());
       }
     }
     if (VALIDATION_DEBUG_WARN) {
       synchronized (System.err) {
-        if (VALIDATION_DEBUG >= 2 || errors.hasProblems() || warnings.hasProblems()) {
+        if (errors.hasProblems()) {
           if (errors.hasProblems()) {
             System.err.println("ERROR: Include(s) were not in declared srcs:");
-          } else if (warnings.hasProblems()) {
-            System.err.println("WARN: Include(s) were not in declared srcs:");
           } else {
             System.err.println("INFO: Include(s) were OK for '" + getSourceFile()
                 + "', declared srcs:");
@@ -803,11 +786,6 @@ public class CppCompileAction extends AbstractAction
           for (PathFragment f : Sets.newTreeSet(ccCompilationContext.getDeclaredIncludeDirs())) {
             System.err.println("  '" + f + "'");
           }
-          System.err.println(" or under declared warn dirs:");
-          for (PathFragment f :
-              Sets.newTreeSet(ccCompilationContext.getDeclaredIncludeWarnDirs())) {
-            System.err.println("  '" + f + "'");
-          }
           System.err.println(" with prefixes:");
           for (PathFragment dirpath : ccCompilationContext.getQuoteIncludeDirs()) {
             System.err.println("  '" + dirpath + "'");
@@ -815,14 +793,6 @@ public class CppCompileAction extends AbstractAction
         }
       }
     }
-
-    if (warnings.hasProblems()) {
-      actionExecutionContext
-          .getEventHandler()
-          .handle(
-              Event.warn(getOwner().getLocation(), warnings.getMessage(this, getSourceFile()))
-                  .withTag(Label.print(getOwner().getLabel())));
-    }
     errors.assertProblemFree(this, getSourceFile());
   }
 
@@ -976,14 +946,6 @@ public class CppCompileAction extends AbstractAction
     return ccCompilationContext.getDeclaredIncludeDirs();
   }
 
-  /**
-   * Return the directories in which to look for headers and issue a warning. (pertains to headers
-   * not specifically listed in {@code declaredIncludeSrcs}).
-   */
-  public NestedSet<PathFragment> getDeclaredIncludeWarnDirs() {
-    return ccCompilationContext.getDeclaredIncludeWarnDirs();
-  }
-
   /** Return explicitly listed header files. */
   @Override
   public NestedSet<Artifact> getDeclaredIncludeSrcs() {
@@ -1029,7 +991,6 @@ public class CppCompileAction extends AbstractAction
      * warning have changed, otherwise we might miss some errors.
      */
     fp.addPaths(ccCompilationContext.getDeclaredIncludeDirs());
-    fp.addPaths(ccCompilationContext.getDeclaredIncludeWarnDirs());
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index b809ed5d30..300a4c9886 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -139,16 +139,24 @@ public final class CppConfiguration extends BuildConfiguration.Fragment
   /**
    * Values for the --hdrs_check option. Note that Bazel only supports and will default to "strict".
    */
-  public static enum HeadersCheckingMode {
+  public enum HeadersCheckingMode {
     /**
      * Legacy behavior: Silently allow any source header file in any of the directories of the
      * containing package to be included by sources in this rule and dependent rules.
      */
     LOOSE,
-    /** Warn about undeclared headers. */
-    WARN,
     /** Disallow undeclared headers. */
-    STRICT
+    STRICT;
+
+    public static HeadersCheckingMode getValue(String value) {
+      if (value.equalsIgnoreCase("loose") || value.equalsIgnoreCase("warn")) {
+        return LOOSE;
+      }
+      if (value.equalsIgnoreCase("strict")) {
+        return STRICT;
+      }
+      throw new IllegalArgumentException();
+    }
   }
 
   /**