GN: (HeaderChecker) Allow any chain to forward direct dependent configs.

tools/gn/header_checker_unittest.cc

Index: tools/gn/header_checker_unittest.cc
diff --git a/tools/gn/header_checker_unittest.cc b/tools/gn/header_checker_unittest.cc
index e916546a6ba66c14e3671dcb35bd58a32ef4ec18..27b92af2206d3a5bce1dd0e5cacf73eba94ecd1a 100644
--- a/tools/gn/header_checker_unittest.cc
+++ b/tools/gn/header_checker_unittest.cc
@@ -5,6 +5,7 @@
 #include <vector>
 
 #include "testing/gtest/include/gtest/gtest.h"
+#include "tools/gn/config.h"
 #include "tools/gn/header_checker.h"
 #include "tools/gn/scheduler.h"
 #include "tools/gn/target.h"
@@ -17,7 +18,8 @@ class HeaderCheckerTest : public testing::Test {
   HeaderCheckerTest()
       : a_(setup_.settings(), Label(SourceDir("//a/"), "a")),
         b_(setup_.settings(), Label(SourceDir("//b/"), "a")),
-        c_(setup_.settings(), Label(SourceDir("//c/"), "c")) {
+        c_(setup_.settings(), Label(SourceDir("//c/"), "c")),
+        d_(setup_.settings(), Label(SourceDir("//d/"), "d")) {
     a_.deps().push_back(LabelTargetPair(&b_));
     b_.deps().push_back(LabelTargetPair(&c_));
 
@@ -25,10 +27,12 @@ class HeaderCheckerTest : public testing::Test {
     a_.visibility().SetPublic();
     b_.visibility().SetPublic();
     c_.visibility().SetPublic();
+    d_.visibility().SetPublic();
 
     targets_.push_back(&a_);
     targets_.push_back(&b_);
     targets_.push_back(&c_);
+    targets_.push_back(&d_);
   }
 
  protected:
@@ -41,6 +45,7 @@ class HeaderCheckerTest : public testing::Test {
   Target a_;
   Target b_;
   Target c_;
+  Target d_;
 
   std::vector<const Target*> targets_;
 };
@@ -52,24 +57,88 @@ TEST_F(HeaderCheckerTest, IsDependencyOf) {
       new HeaderChecker(setup_.build_settings(), targets_));
 
   std::vector<const Target*> chain;
-  EXPECT_FALSE(checker->IsDependencyOf(&a_, &a_, &chain));
+  EXPECT_FALSE(checker->IsDependencyOf(&a_, &a_, false, &chain, NULL));
 
   chain.clear();
-  EXPECT_TRUE(checker->IsDependencyOf(&b_, &a_, &chain));
+  EXPECT_TRUE(checker->IsDependencyOf(&b_, &a_, false, &chain, NULL));
   ASSERT_EQ(2u, chain.size());
   EXPECT_EQ(&b_, chain[0]);
   EXPECT_EQ(&a_, chain[1]);
 
   chain.clear();
-  EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, &chain));
+  EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, false, &chain, NULL));
   ASSERT_EQ(3u, chain.size());
   EXPECT_EQ(&c_, chain[0]);
   EXPECT_EQ(&b_, chain[1]);
   EXPECT_EQ(&a_, chain[2]);
 
   chain.clear();
-  EXPECT_FALSE(checker->IsDependencyOf(&a_, &c_, &chain));
+  EXPECT_FALSE(checker->IsDependencyOf(&a_, &c_, false, &chain, NULL));
   EXPECT_TRUE(chain.empty());
+
+  // If an a -> c dependency exists, this should be chosen for the chain.
+  chain.clear();
+  a_.deps().push_back(LabelTargetPair(&c_));
+  EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, false, &chain, NULL));
+  EXPECT_EQ(&c_, chain[0]);
+  EXPECT_EQ(&a_, chain[1]);
+}
+
+TEST_F(HeaderCheckerTest, IsDependencyOf_ForwardsDirectDependentConfigs) {
+  scoped_refptr<HeaderChecker> checker(
+      new HeaderChecker(setup_.build_settings(), targets_));
+
+  // The a -> b -> c chain is found, since no chains that forward direct-
+  // dependent configs exist.
+  std::vector<const Target*> chain;
+  bool direct_dependent_configs_apply = false;
+  EXPECT_TRUE(checker->IsDependencyOf(
+      &c_, &a_, true, &chain, &direct_dependent_configs_apply));
+  EXPECT_FALSE(direct_dependent_configs_apply);
+  EXPECT_EQ(3u, chain.size());
+  EXPECT_EQ(&c_, chain[0]);
+  EXPECT_EQ(&b_, chain[1]);
+  EXPECT_EQ(&a_, chain[2]);
+
+  // Create a chain a -> d -> c where d forwards direct-dependent configs.
+  // This path should be preferred when dependency chains which forward
+  // direct-dependent configs are preferred.
+  chain.clear();
+  direct_dependent_configs_apply = false;
+  d_.deps().push_back(LabelTargetPair(&c_));
+  d_.forward_dependent_configs().push_back(LabelTargetPair(&c_));
+  a_.deps().push_back(LabelTargetPair(&d_));
+  EXPECT_TRUE(checker->IsDependencyOf(
+      &c_, &a_, true, &chain, &direct_dependent_configs_apply));
+  EXPECT_TRUE(direct_dependent_configs_apply);
+  EXPECT_EQ(3u, chain.size());
+  EXPECT_EQ(&c_, chain[0]);
+  EXPECT_EQ(&d_, chain[1]);
+  EXPECT_EQ(&a_, chain[2]);
+
+  // d also forwards direct-dependent configs if it is a group.
+  chain.clear();
+  direct_dependent_configs_apply = false;
+  d_.set_output_type(Target::GROUP);
+  d_.forward_dependent_configs().clear();
+  EXPECT_TRUE(checker->IsDependencyOf(
+      &c_, &a_, true, &chain, &direct_dependent_configs_apply));
+  EXPECT_TRUE(direct_dependent_configs_apply);
+  EXPECT_EQ(3u, chain.size());
+  EXPECT_EQ(&c_, chain[0]);
+  EXPECT_EQ(&d_, chain[1]);
+  EXPECT_EQ(&a_, chain[2]);
+
+  // A direct dependency a -> c carries direct-dependent configs.
+  chain.clear();
+  direct_dependent_configs_apply = false;
+  a_.deps().push_back(LabelTargetPair(&c_));
+  EXPECT_TRUE(checker->IsDependencyOf(
+      &c_, &a_, true, &chain, &direct_dependent_configs_apply));
+  EXPECT_TRUE(direct_dependent_configs_apply);
+  EXPECT_EQ(2u, chain.size());
+  EXPECT_EQ(&c_, chain[0]);
+  EXPECT_EQ(&a_, chain[1]);
 }
 
 TEST_F(HeaderCheckerTest, CheckInclude) {
@@ -79,9 +148,8 @@ TEST_F(HeaderCheckerTest, CheckInclude) {
 
   // Add a disconnected target d with a header to check that you have to have
   // to depend on a target listing a header.
-  Target d(setup_.settings(), Label(SourceDir("//"), "d"));
   SourceFile d_header("//d_header.h");
-  d.sources().push_back(SourceFile(d_header));
+  d_.sources().push_back(SourceFile(d_header));
 
   // Add a header on B and say everything in B is public.
   SourceFile b_public("//b_public.h");
@@ -95,7 +163,7 @@ TEST_F(HeaderCheckerTest, CheckInclude) {
   c_.public_headers().push_back(c_public);
   c_.set_all_headers_public(false);
 
-  targets_.push_back(&d);
+  targets_.push_back(&d_);
   scoped_refptr<HeaderChecker> checker(
       new HeaderChecker(setup_.build_settings(), targets_));
 
@@ -129,9 +197,36 @@ TEST_F(HeaderCheckerTest, CheckInclude) {
   err = Err();
   EXPECT_FALSE(checker->CheckInclude(&a_, input_file, c_public, range, &err));
   EXPECT_TRUE(err.has_error());
+  c_.visibility().SetPublic();
+
+  // If C has direct-dependent configs, then B must forward them to A.
+  // If B is a group, that suffices to forward direct-dependent configs.
+  {
+    Config direct(setup_.settings(), Label(SourceDir("//c/"), "config"));
+    direct.config_values().cflags().push_back("-DSOME_DEFINE");
+
+    c_.direct_dependent_configs().push_back(LabelConfigPair(&direct));
+    err = Err();
+    EXPECT_FALSE(checker->CheckInclude(&a_, input_file, c_public, range, &err));
+    EXPECT_TRUE(err.has_error());
+
+    b_.forward_dependent_configs().push_back(LabelTargetPair(&c_));
+    err = Err();
+    EXPECT_TRUE(checker->CheckInclude(&a_, input_file, c_public, range, &err));
+    EXPECT_FALSE(err.has_error());
+
+    b_.forward_dependent_configs().clear();
+    b_.set_output_type(Target::GROUP);
+    err = Err();
+    EXPECT_TRUE(checker->CheckInclude(&a_, input_file, c_public, range, &err));
+    EXPECT_FALSE(err.has_error());
+
+    b_.set_output_type(Target::UNKNOWN);
+    c_.direct_dependent_configs().clear();
+  }
 }
 
-TEST_F(HeaderCheckerTest, DoDirectDependentConfigsApply) {
+TEST_F(HeaderCheckerTest, GetDependentConfigChainProblemIndex) {
   // Assume we have a chain A -> B -> C -> D.
   Target target_a(setup_.settings(), Label(SourceDir("//a/"), "a"));
   Target target_b(setup_.settings(), Label(SourceDir("//b/"), "b"));
@@ -153,19 +248,12 @@ TEST_F(HeaderCheckerTest, DoDirectDependentConfigsApply) {
   chain.push_back(&target_b);
   chain.push_back(&target_a);
 
-  // This chain should be valid.
-  size_t badone = 0;
-  EXPECT_TRUE(HeaderChecker::DoDirectDependentConfigsApply(chain, &badone));
-
   // If C is not a group, it shouldn't work anymore.
   target_c.set_output_type(Target::SOURCE_SET);
-  EXPECT_FALSE(HeaderChecker::DoDirectDependentConfigsApply(chain, &badone));
-  EXPECT_EQ(1u, badone);
+  EXPECT_EQ(1u, HeaderChecker::GetDependentConfigChainProblemIndex(chain));
 
   // Or if B stops forwarding from C, it shouldn't work anymore.
   target_c.set_output_type(Target::GROUP);
-  badone = 0;
   target_b.forward_dependent_configs().clear();
-  EXPECT_FALSE(HeaderChecker::DoDirectDependentConfigsApply(chain, &badone));
-  EXPECT_EQ(2u, badone);
+  EXPECT_EQ(2u, HeaderChecker::GetDependentConfigChainProblemIndex(chain));
 }