Fix variable use tracking for scope subscript accesses.
Accessing a scope field with a subscript expression as in:
scope["foo"]
Did not mark the 'foo' field in the scope as used, unlike
the corresponding expression `scope.foo`.
CL fixes that sharing the logic for scope member evaluation
between AccessorNode::ExecuteScopeSubscriptAccess() and
AccessorNode::ExecuteScopeAccess().
Change-Id: I97b0efa5a2d24c6a99b1a70a33626d1860747909
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/15920
Commit-Queue: David Turner <digit@google.com>
Reviewed-by: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/parse_tree.cc b/src/gn/parse_tree.cc
index 28d5313..0f1c27e 100644
--- a/src/gn/parse_tree.cc
+++ b/src/gn/parse_tree.cc
@@ -391,7 +391,7 @@
if (!key_value.VerifyTypeIs(Value::STRING, err))
return Value();
const Value* result =
- base_value->scope_value()->GetValue(key_value.string_value());
+ ExecuteScopeAccessForMember(scope, key_value.string_value(), err);
if (!result) {
*err =
Err(subscript_.get(), "No value named \"" + key_value.string_value() +
@@ -402,6 +402,21 @@
}
Value AccessorNode::ExecuteScopeAccess(Scope* scope, Err* err) const {
+ const Value* result =
+ ExecuteScopeAccessForMember(scope, member_->value().value(), err);
+
+ if (!result) {
+ *err = Err(member_.get(), "No value named \"" + member_->value().value() +
+ "\" in scope \"" + base_.value() + "\"");
+ return Value();
+ }
+ return *result;
+}
+
+const Value* AccessorNode::ExecuteScopeAccessForMember(
+ Scope* scope,
+ std::string_view member_str,
+ Err* err) const {
// We jump through some hoops here since ideally a.b will count "b" as
// accessed in the given scope. The value "a" might be in some normal nested
// scope and we can modify it, but it might also be inherited from the
@@ -418,30 +433,22 @@
// Common case: base value is mutable so we can track variable accesses
// for unused value warnings.
if (!mutable_base_value->VerifyTypeIs(Value::SCOPE, err))
- return Value();
- result = mutable_base_value->scope_value()->GetValue(
- member_->value().value(), true);
+ return nullptr;
+ result = mutable_base_value->scope_value()->GetValue(member_str, true);
} else {
// Fall back to see if the value is on a read-only scope.
const Value* const_base_value = scope->GetValue(base_.value(), true);
if (const_base_value) {
// Read only value, don't try to mark the value access as a "used" one.
if (!const_base_value->VerifyTypeIs(Value::SCOPE, err))
- return Value();
- result =
- const_base_value->scope_value()->GetValue(member_->value().value());
+ return nullptr;
+ result = const_base_value->scope_value()->GetValue(member_str);
} else {
*err = Err(base_, "Undefined identifier.");
- return Value();
+ return nullptr;
}
}
-
- if (!result) {
- *err = Err(member_.get(), "No value named \"" + member_->value().value() +
- "\" in scope \"" + base_.value() + "\"");
- return Value();
- }
- return *result;
+ return result;
}
void AccessorNode::SetNewLocation(int line_number) {
diff --git a/src/gn/parse_tree.h b/src/gn/parse_tree.h
index 7922cd6..2ed8c54 100644
--- a/src/gn/parse_tree.h
+++ b/src/gn/parse_tree.h
@@ -206,6 +206,9 @@
const Value* base_value,
Err* err) const;
Value ExecuteScopeAccess(Scope* scope, Err* err) const;
+ const Value* ExecuteScopeAccessForMember(Scope* scope,
+ std::string_view member_str,
+ Err* err) const;
static constexpr const char* kDumpAccessorKind = "accessor_kind";
static constexpr const char* kDumpAccessorKindSubscript = "subscript";
diff --git a/src/gn/parse_tree_unittest.cc b/src/gn/parse_tree_unittest.cc
index cc49848..9b684f0 100644
--- a/src/gn/parse_tree_unittest.cc
+++ b/src/gn/parse_tree_unittest.cc
@@ -83,15 +83,20 @@
EXPECT_EQ(second.type(), Value::INTEGER);
EXPECT_EQ(second.int_value(), 3);
+ const Scope* scope_var = setup.scope()->GetValue("scope")->scope_value();
+ EXPECT_TRUE(scope_var->IsSetButUnused("foo"));
Value foo = setup.ExecuteExpression("scope[\"foo\"]", &err);
EXPECT_FALSE(err.has_error());
EXPECT_EQ(foo.type(), Value::INTEGER);
EXPECT_EQ(foo.int_value(), 5);
+ EXPECT_FALSE(scope_var->IsSetButUnused("foo"));
+ EXPECT_TRUE(scope_var->IsSetButUnused("bar"));
Value bar = setup.ExecuteExpression("scope[bar_key]", &err);
EXPECT_FALSE(err.has_error());
EXPECT_EQ(bar.type(), Value::INTEGER);
EXPECT_EQ(bar.int_value(), 8);
+ EXPECT_FALSE(scope_var->IsSetButUnused("bar"));
Value invalid1 = setup.ExecuteExpression("scope[second_element_idx]", &err);
EXPECT_TRUE(err.has_error());
