commit 230edb88b12d79f8719562e930fb76ef8229052a
author brettw <brettw@chromium.org> Wed Oct 05 11:25:08 2016 -0700
committer Commit bot <commit-bot@chromium.org> Wed Oct 05 18:28:10 2016 +0000
tree 7d3b84089ac48c489445f29b0ddae768230ab0fe
parent e94fcff80395a904c60eef1ed05cbd7dc7953dbd

GN: Mark variables in nested scopes used for += and -=.

Previuosly GN would give an unused variable warning when you did += in an inner
scope for a variable defined in an outer scope. This patch marks the outer
variable used in this case.

Review-Url: https://codereview.chromium.org/2393853003
Cr-Original-Commit-Position: refs/heads/master@{#423226}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 80be99d1402bc09c858868b66761d2df77f218b3

tools/gn/operators.cc

diff --git a/tools/gn/operators.cc b/tools/gn/operators.cc
index 9ac18bf..72b2dd1 100644
--- a/tools/gn/operators.cc
+++ b/tools/gn/operators.cc
@@ -155,7 +155,7 @@
 
 const Value* ValueDestination::GetExistingValue() const {
   if (type_ == SCOPE)
-    return scope_->GetValue(name_token_->value(), false);
+    return scope_->GetValue(name_token_->value(), true);
   else if (type_ == LIST)
     return &list_->list_value()[index_];
   return nullptr;

tools/gn/operators_unittest.cc

diff --git a/tools/gn/operators_unittest.cc b/tools/gn/operators_unittest.cc
index 4fa3d11..fa163a1 100644
--- a/tools/gn/operators_unittest.cc
+++ b/tools/gn/operators_unittest.cc
@@ -330,3 +330,34 @@
   ASSERT_EQ(Value::SCOPE, new_value->type());
   ASSERT_FALSE(new_value->scope_value()->HasValues(Scope::SEARCH_CURRENT));
 }
+
+// Tests this case:
+//  foo = 1
+//  target(...) {
+//    foo += 1
+//
+// This should mark the outer "foo" as used, and the inner "foo" as unused.
+TEST(Operators, PlusEqualsUsed) {
+  Err err;
+  TestWithScope setup;
+
+  // Outer "foo" definition, it should be unused.
+  const char foo[] = "foo";
+  Value old_value(nullptr, static_cast<int64_t>(1));
+  setup.scope()->SetValue(foo, old_value, nullptr);
+  EXPECT_TRUE(setup.scope()->IsSetButUnused(foo));
+
+  // Nested scope.
+  Scope nested(setup.scope());
+
+  // Run "foo += 1".
+  TestBinaryOpNode node(Token::PLUS_EQUALS, "+=");
+  node.SetLeftToIdentifier(foo);
+  node.SetRightToValue(Value(nullptr, static_cast<int64_t>(1)));
+  node.Execute(&nested, &err);
+  ASSERT_FALSE(err.has_error());
+
+  // Outer foo should be used, inner foo should not be.
+  EXPECT_FALSE(setup.scope()->IsSetButUnused(foo));
+  EXPECT_TRUE(nested.IsSetButUnused(foo));
+}