Commit ea4a89da authored by brettw@chromium.org's avatar brettw@chromium.org

Improve GN value origins for error messages.

Sets the origin for a value when the value is dereferenced. Previously if you did
  foo = 6
  deps = [ foo ]
You would get the message:
  ERROR at //BUILD.gn:15:9: This is not a string.
    foo = 6
          ^
Which in this case is pretty confusing since that's obviously the case, and you really want to know where it was dereferenced.

With this patch the message is:
  ERROR at //BUILD.gn:21:5: This is not a string.
      foo,
      ^--
  Instead I see a integer = 6
which is more actionable.

R=ajwong@chromium.org

Review URL: https://codereview.chromium.org/439513003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288131 0039d316-1c4b-4281-b951-d872f2087c98
parent 3b6a192b
...@@ -373,12 +373,16 @@ const IdentifierNode* IdentifierNode::AsIdentifier() const { ...@@ -373,12 +373,16 @@ const IdentifierNode* IdentifierNode::AsIdentifier() const {
} }
Value IdentifierNode::Execute(Scope* scope, Err* err) const { Value IdentifierNode::Execute(Scope* scope, Err* err) const {
const Value* result = scope->GetValue(value_.value(), true); const Value* value = scope->GetValue(value_.value(), true);
if (!result) { Value result;
if (!value) {
*err = MakeErrorDescribing("Undefined identifier"); *err = MakeErrorDescribing("Undefined identifier");
return Value(); return result;
} }
return *result;
result = *value;
result.set_origin(this);
return result;
} }
LocationRange IdentifierNode::GetRange() const { LocationRange IdentifierNode::GetRange() const {
......
...@@ -76,4 +76,28 @@ TEST(ParseTree, BlockUnusedVars) { ...@@ -76,4 +76,28 @@ TEST(ParseTree, BlockUnusedVars) {
input_unused.parsed()->Execute(setup.scope(), &err); input_unused.parsed()->Execute(setup.scope(), &err);
EXPECT_TRUE(err.has_error()); EXPECT_TRUE(err.has_error());
// Also verify that the unused variable has the correct origin set. The
// origin will point to the value assigned to the variable (in this case, the
// "13" assigned to "b".
EXPECT_EQ(3, err.location().line_number());
EXPECT_EQ(7, err.location().char_offset());
}
TEST(ParseTree, OriginForDereference) {
TestWithScope setup;
TestParseInput input(
"a = 6\n"
"get_target_outputs(a)");
EXPECT_FALSE(input.has_error());
Err err;
input.parsed()->Execute(setup.scope(), &err);
EXPECT_TRUE(err.has_error());
// The origin for the "not a string" error message should be where the value
// was dereferenced (the "a" on the second line).
EXPECT_EQ(2, err.location().line_number());
EXPECT_EQ(20, err.location().char_offset());
} }
...@@ -177,7 +177,10 @@ bool Value::VerifyTypeIs(Type t, Err* err) const { ...@@ -177,7 +177,10 @@ bool Value::VerifyTypeIs(Type t, Err* err) const {
if (type_ == t) if (type_ == t)
return true; return true;
*err = Err(origin(), std::string("This is not a ") + DescribeType(t) + "."); *err = Err(origin(),
std::string("This is not a ") + DescribeType(t) + ".",
std::string("Instead I see a ") + DescribeType(type_) + " = " +
ToString(true));
return false; return false;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment