GN foreach should mark the list identifier as used.
Previously an identifier used as the list in a feoreach statement would not count as "using" the identifier, which would give unused variable errors. Added a unit test.
Checks for unused identifiers in component(). Previously these were not checked. This checking turned up a misspelling in the cc build. Added a unit test.
Fixes for the ozone media build (turned up when testing).
R=dpranke@chromium.org
Review URL: https://codereview.chromium.org/420443003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285367 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/cc/BUILD.gn b/cc/BUILD.gn
index 79b22a9..1c04a89 100644
--- a/cc/BUILD.gn
+++ b/cc/BUILD.gn
@@ -473,7 +473,7 @@
"//skia",
]
- defined = [ "CC_IMPLEMENTATION=1" ]
+ defines = [ "CC_IMPLEMENTATION=1" ]
if (!is_debug && is_win) {
configs -= [ "//build/config/compiler:optimize" ]
diff --git a/media/BUILD.gn b/media/BUILD.gn
index aa7daea..f2acd9205 100644
--- a/media/BUILD.gn
+++ b/media/BUILD.gn
@@ -42,6 +42,28 @@
}
}
+if (use_ozone) {
+ action("generate_ozone_constructor_list") {
+ # Ozone platform objects are auto-generated using similar
+ # patterns for naming and classes constructors. Here we build the
+ # object MediaOzonePlatform.
+ script = "../ui/ozone/generate_constructor_list.py"
+
+ platform_list_txt_file = "$target_gen_dir/ui/ozone/platform_list.txt"
+ constructor_list_cc_file = "$target_gen_dir/media/ozone/constructor_list.cc"
+
+ sources = [ platform_list_txt_file ]
+ outputs = [ constructor_list_cc_file ]
+ args = [
+ "--platform_list=$platform_list_txt_file",
+ "--output_cc=$constructor_list_cc_file",
+ "--namespace=media",
+ "--typename=MediaOzonePlatform",
+ "--include=\"media/ozone/media_ozone_platform.h\""
+ ]
+ }
+}
+
component("media") {
sources = [
"base/audio_block_fifo.cc",
@@ -506,35 +528,18 @@
}
if (use_ozone) {
- platform_list_txt_file = "$target_gen_dir/ui/ozone/platform_list.txt"
- constructor_list_cc_file = "$target_gen_dir/media/ozone/constructor_list.cc"
-
# Used for the generated listing header (ui/ozone/platform_list.h)
include_dirs += [ target_gen_dir ]
sources += [
- constructor_list_cc_file,
"ozone/media_ozone_platform.cc",
"ozone/media_ozone_platform.h",
+ ] + get_target_outputs(":generate_ozone_constructor_list")
+
+ deps += [
+ ":generate_ozone_constructor_list",
+ "//ui/ozone",
]
-
- deps += [ "//ui/ozone/ozone" ]
-
- action("generate_constructor_list") {
- # Ozone platform objects are auto-generated using similar
- # patterns for naming and classes constructors. Here we build the
- # object MediaOzonePlatform.
- script = "../ui/ozone/generate_constructor_list.py"
- sources = [ platform_list_txt_file ]
- outputs = [ constructor_list_cc_file ]
- args += [
- "--platform_list=$platform_list_txt_file",
- "--output_cc=$constructor_list_cc_file",
- "--namespace=media",
- "--typename=MediaOzonePlatform",
- "--include=\"media/ozone/media_ozone_platform.h\""
- ]
- }
}
if (is_mac) {
diff --git a/tools/gn/BUILD.gn b/tools/gn/BUILD.gn
index 87ec6c6..66bbf2f 100644
--- a/tools/gn/BUILD.gn
+++ b/tools/gn/BUILD.gn
@@ -187,6 +187,7 @@
"function_get_target_outputs_unittest.cc",
"function_rebase_path_unittest.cc",
"function_write_file_unittest.cc",
+ "functions_target_unittest.cc",
"functions_unittest.cc",
"header_checker_unittest.cc",
"input_conversion_unittest.cc",
diff --git a/tools/gn/function_foreach.cc b/tools/gn/function_foreach.cc
index 40f3892..06a2dc1 100644
--- a/tools/gn/function_foreach.cc
+++ b/tools/gn/function_foreach.cc
@@ -70,7 +70,7 @@
const Value* list_value = NULL;
const IdentifierNode* list_identifier = args_vector[1]->AsIdentifier();
if (list_identifier) {
- list_value = scope->GetValue(list_identifier->value().value());
+ list_value = scope->GetValue(list_identifier->value().value(), true);
if (!list_value) {
*err = Err(args_vector[1], "Undefined identifier.");
return Value();
diff --git a/tools/gn/function_foreach_unittest.cc b/tools/gn/function_foreach_unittest.cc
index 15ca8a4..462d714 100644
--- a/tools/gn/function_foreach_unittest.cc
+++ b/tools/gn/function_foreach_unittest.cc
@@ -51,3 +51,25 @@
input_bad.parsed()->Execute(setup.scope(), &err);
ASSERT_TRUE(err.has_error()); // Shouldn't actually run.
}
+
+// Checks that the identifier used as the list is marked as "used".
+TEST(FunctionForeach, MarksIdentAsUsed) {
+ TestWithScope setup;
+ TestParseInput input_good(
+ "a = [1, 2]\n"
+ "foreach(i, a) {\n"
+ " print(i)\n"
+ "}\n");
+ ASSERT_FALSE(input_good.has_error());
+
+ Err err;
+ input_good.parsed()->Execute(setup.scope(), &err);
+ ASSERT_FALSE(err.has_error()) << err.message();
+
+ EXPECT_EQ("1\n2\n", setup.print_output());
+ setup.print_output().clear();
+
+ // Check for unused vars.
+ EXPECT_TRUE(setup.scope()->CheckForUnusedVars(&err));
+ EXPECT_FALSE(err.has_error());
+}
diff --git a/tools/gn/functions_target.cc b/tools/gn/functions_target.cc
index 7e952f1..8aea0128 100644
--- a/tools/gn/functions_target.cc
+++ b/tools/gn/functions_target.cc
@@ -292,6 +292,8 @@
TargetGenerator::GenerateTarget(&block_scope, function, args,
component_mode, err);
+
+ block_scope.CheckForUnusedVars(err);
return Value();
}
diff --git a/tools/gn/functions_target_unittest.cc b/tools/gn/functions_target_unittest.cc
new file mode 100644
index 0000000..c8f6176
--- /dev/null
+++ b/tools/gn/functions_target_unittest.cc
@@ -0,0 +1,50 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "tools/gn/scheduler.h"
+#include "tools/gn/scope.h"
+#include "tools/gn/test_with_scope.h"
+
+
+// Checks that we find unused identifiers in targets.
+TEST(FunctionsTarget, CheckUnused) {
+ Scheduler scheduler;
+ TestWithScope setup;
+
+ // The target generator needs a place to put the targets or it will fail.
+ Scope::ItemVector item_collector;
+ setup.scope()->set_item_collector(&item_collector);
+
+ // Test a good one first.
+ TestParseInput good_input(
+ "source_set(\"foo\") {\n"
+ "}\n");
+ ASSERT_FALSE(good_input.has_error());
+ Err err;
+ good_input.parsed()->Execute(setup.scope(), &err);
+ ASSERT_FALSE(err.has_error()) << err.message();
+
+ // Test a source set (this covers everything but component) with an unused
+ // variable.
+ TestParseInput source_set_input(
+ "source_set(\"foo\") {\n"
+ " unused = 5\n"
+ "}\n");
+ ASSERT_FALSE(source_set_input.has_error());
+ err = Err();
+ source_set_input.parsed()->Execute(setup.scope(), &err);
+ ASSERT_TRUE(err.has_error());
+
+ // Test a component, which is a different code path.
+ TestParseInput component_input(
+ "component_mode = \"static_library\"\n"
+ "component(\"bar\") {\n"
+ " unused = 5\n"
+ "}\n");
+ ASSERT_FALSE(component_input.has_error());
+ err = Err();
+ component_input.parsed()->Execute(setup.scope(), &err);
+ ASSERT_TRUE(err.has_error()) << err.message();
+}
diff --git a/tools/gn/gn.gyp b/tools/gn/gn.gyp
index d972b5b..c94c8e2 100644
--- a/tools/gn/gn.gyp
+++ b/tools/gn/gn.gyp
@@ -188,6 +188,7 @@
'function_get_target_outputs_unittest.cc',
'function_rebase_path_unittest.cc',
'function_write_file_unittest.cc',
+ 'functions_target_unittest.cc',
'functions_unittest.cc',
'header_checker_unittest.cc',
'input_conversion_unittest.cc',