Remove support for set_sources_assignment_filter function
Mark the function deprecated and print an error if it is called with
a non-empty list as a parameter. This is a preliminary until all the
no-op uses of set_sources_assignment_filter([]) have been removed.
Bug: 125
Change-Id: Ia63e1135202a2921de791135a2b4b2eb6ac3861b
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/10242
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
diff --git a/docs/language.md b/docs/language.md
index 28bdb7a..137300a 100644
--- a/docs/language.md
+++ b/docs/language.md
@@ -58,11 +58,6 @@
per the above design philosophy, if you need this kind of thing you're probably
doing it wrong.
-The variable `sources` has a special rule: when assigning to it, a list
-of exclusion patterns is applied to it. This is designed to
-automatically filter out some types of files. See `gn help
-set_sources_assignment_filter` and `gn help label_pattern` for more.
-
The full grammar for language nerds is available in `gn help grammar`.
### Strings
@@ -471,7 +466,7 @@
Patterns are used to generate the output file names for a given set of
inputs for custom target types, and to automatically remove files from
-the `sources` variable (see `gn help set_sources_assignment_filter`).
+the list values (see `gn help filter_include` and `gn help filter_exclude`).
They are like simple regular expressions. See `gn help label_pattern`
for more.
diff --git a/docs/reference.md b/docs/reference.md
index 93cba60..2a987ba 100644
--- a/docs/reference.md
+++ b/docs/reference.md
@@ -57,7 +57,7 @@
* [rebase_path: Rebase a file or directory to another location.](#func_rebase_path)
* [set_default_toolchain: Sets the default toolchain name.](#func_set_default_toolchain)
* [set_defaults: Set default values for a target type.](#func_set_defaults)
- * [set_sources_assignment_filter: Set a pattern to filter source files.](#func_set_sources_assignment_filter)
+ * [set_sources_assignment_filter: Deprecated feature.](#func_set_sources_assignment_filter)
* [split_list: Splits a list into N different sub-lists.](#func_split_list)
* [string_join: Concatenates a list of strings with a separator.](#func_string_join)
* [string_replace: Replaces substring in the given string.](#func_string_replace)
@@ -2424,10 +2424,6 @@
important because most targets have an implicit configs list, which means it
wouldn't work at all if it didn't clobber).
- The sources assignment filter (see "gn help set_sources_assignment_filter")
- is never applied by this function. It's assumed than any desired filtering
- was already done when sources was set on the from_scope.
-
If variables_to_not_forward_list is non-empty, then it must contains a list
of variable names that will not be forwarded. This is mostly useful when
variable_list_or_star has a value of "*".
@@ -3055,36 +3051,11 @@
configs -= [ "//tools/mything:settings" ]
}
```
-### <a name="func_set_sources_assignment_filter"></a>**set_sources_assignment_filter**: Set a pattern to filter source files.
+### <a name="func_set_sources_assignment_filter"></a>**set_sources_assignment_filter**: Deprecated feature.
```
- The sources assignment filter is a list of patterns that remove files from
- the list implicitly whenever the "sources" variable is assigned to. This will
- do nothing for non-lists.
-
- This is intended to be used to globally filter out files with
- platform-specific naming schemes when they don't apply, for example you may
- want to filter out all "*_win.cc" files on non-Windows platforms.
-
- Typically this will be called once in the master build config script to set
- up the filter for the current platform. Subsequent calls will overwrite the
- previous values.
-
- If you want to bypass the filter and add a file even if it might be filtered
- out, call set_sources_assignment_filter([]) to clear the list of filters.
- This will apply until the current scope exits.
-
- See "gn help file_pattern" for more information on file pattern.
-```
-
-#### **Sources assignment example**
-
-```
- # Filter out all _win files.
- set_sources_assignment_filter([ "*_win.cc", "*_win.h" ])
- sources = [ "a.cc", "b_win.cc" ]
- print(sources)
- # Will print [ "a.cc" ]. b_win one was filtered out.
+ This feature is deprecated. It will be removed once all usages have been
+ removed. Only supports a single argument that needs to be an empty list.
```
### <a name="func_split_list"></a>**split_list**: Splits a list into N different sub-lists.
@@ -7078,10 +7049,6 @@
mylist = []
mylist = otherlist
-
- When assigning to a list named 'sources' using '=' or '+=', list items may be
- automatically filtered out. See "gn help set_sources_assignment_filter" for
- more.
```
#### **Scopes**
diff --git a/misc/emacs/gn-mode.el b/misc/emacs/gn-mode.el
index d7bf4bb..dc92c9b 100644
--- a/misc/emacs/gn-mode.el
+++ b/misc/emacs/gn-mode.el
@@ -71,9 +71,8 @@
"forward_variables_from" "get_label_info" "get_path_info"
"get_target_outputs" "getenv" "import" "not_needed" "print"
"process_file_template" "read_file" "rebase_path" "set_default_toolchain"
- "set_defaults" "set_sources_assignment_filter" "split_list" "string_join"
- "string_split" "template" "tool" "toolchain" "propagates_configs"
- "write_file"))
+ "set_defaults" "split_list" "string_join" "string_split" "template" "tool"
+ "toolchain" "propagates_configs" "write_file"))
(defvar gn-font-lock-predefined-var-keywords
'("current_cpu" "current_os" "current_toolchain" "default_toolchain"
diff --git a/misc/tm/GN.tmLanguage b/misc/tm/GN.tmLanguage
index 5f07e21..8be5f56 100644
--- a/misc/tm/GN.tmLanguage
+++ b/misc/tm/GN.tmLanguage
@@ -73,7 +73,7 @@
<key>comment</key>
<string>functions</string>
<key>match</key>
- <string>\b(?:assert|config|declare_args|defined|exec_script|foreach|get_label_info|get_path_info|get_target_outputs|getenv|import|print|process_file_template|read_file|rebase_path|set_default_toolchain|set_defaults|set_sources_assignment_filter|split_list|string_join|string_split|template|tool|toolchain|toolchain_args|propagates_configs|write_file)\b</string>
+ <string>\b(?:assert|config|declare_args|defined|exec_script|foreach|get_label_info|get_path_info|get_target_outputs|getenv|import|print|process_file_template|read_file|rebase_path|set_default_toolchain|set_defaults|split_list|string_join|string_split|template|tool|toolchain|toolchain_args|propagates_configs|write_file)\b</string>
<key>name</key>
<string>entity.name.function.gn</string>
</dict>
diff --git a/misc/vim/syntax/gn.vim b/misc/vim/syntax/gn.vim
index 550f39c..454aacc 100644
--- a/misc/vim/syntax/gn.vim
+++ b/misc/vim/syntax/gn.vim
@@ -37,9 +37,8 @@
syn keyword gnFunctions get_target_outputs getenv import print
syn keyword gnFunctions process_file_template propagates_configs read_file
syn keyword gnFunctions rebase_path set_default_toolchain set_defaults
-syn keyword gnFunctions set_sources_assignment_filter split_list string_join
-syn keyword gnFunctions string_split template tool toolchain toolchain_args
-syn keyword gnFunctions write_file
+syn keyword gnFunctions split_list string_join string_split template tool
+syn keyword gnFunctions toolchain toolchain_args write_file
hi def link gnFunctions Macro
" Variables
diff --git a/src/gn/function_forward_variables_from.cc b/src/gn/function_forward_variables_from.cc
index 8cee386..ba392e1 100644
--- a/src/gn/function_forward_variables_from.cc
+++ b/src/gn/function_forward_variables_from.cc
@@ -108,10 +108,6 @@
important because most targets have an implicit configs list, which means it
wouldn't work at all if it didn't clobber).
- The sources assignment filter (see "gn help set_sources_assignment_filter")
- is never applied by this function. It's assumed than any desired filtering
- was already done when sources was set on the from_scope.
-
If variables_to_not_forward_list is non-empty, then it must contains a list
of variable names that will not be forwarded. This is mostly useful when
variable_list_or_star has a value of "*".
diff --git a/src/gn/functions.cc b/src/gn/functions.cc
index 314173b..e627e2a 100644
--- a/src/gn/functions.cc
+++ b/src/gn/functions.cc
@@ -810,35 +810,12 @@
const char kSetSourcesAssignmentFilter[] = "set_sources_assignment_filter";
const char kSetSourcesAssignmentFilter_HelpShort[] =
- "set_sources_assignment_filter: Set a pattern to filter source files.";
+ "set_sources_assignment_filter: Deprecated feature.";
const char kSetSourcesAssignmentFilter_Help[] =
- R"(set_sources_assignment_filter: Set a pattern to filter source files.
+ R"(set_sources_assignment_filter: Deprecated feature.
- The sources assignment filter is a list of patterns that remove files from
- the list implicitly whenever the "sources" variable is assigned to. This will
- do nothing for non-lists.
-
- This is intended to be used to globally filter out files with
- platform-specific naming schemes when they don't apply, for example you may
- want to filter out all "*_win.cc" files on non-Windows platforms.
-
- Typically this will be called once in the master build config script to set
- up the filter for the current platform. Subsequent calls will overwrite the
- previous values.
-
- If you want to bypass the filter and add a file even if it might be filtered
- out, call set_sources_assignment_filter([]) to clear the list of filters.
- This will apply until the current scope exits.
-
- See "gn help file_pattern" for more information on file pattern.
-
-Sources assignment example
-
- # Filter out all _win files.
- set_sources_assignment_filter([ "*_win.cc", "*_win.h" ])
- sources = [ "a.cc", "b_win.cc" ]
- print(sources)
- # Will print [ "a.cc" ]. b_win one was filtered out.
+ This feature is deprecated. It will be removed once all usages have been
+ removed. Only supports a single argument that needs to be an empty list.
)";
Value RunSetSourcesAssignmentFilter(Scope* scope,
@@ -847,12 +824,19 @@
Err* err) {
if (args.size() != 1) {
*err = Err(function, "set_sources_assignment_filter takes one argument.");
- } else {
- std::unique_ptr<PatternList> f = std::make_unique<PatternList>();
- f->SetFromValue(args[0], err);
- if (!err->has_error())
- scope->set_sources_assignment_filter(std::move(f));
+ return Value();
}
+
+ if (!args[0].VerifyTypeIs(Value::LIST, err)) {
+ return Value();
+ }
+
+ if (!args[0].list_value().empty()) {
+ *err = Err(function,
+ "set_sources_assignment_filter argument must be an empty list.");
+ return Value();
+ }
+
return Value();
}
diff --git a/src/gn/operators.cc b/src/gn/operators.cc
index 3c55a90..7c04dd3 100644
--- a/src/gn/operators.cc
+++ b/src/gn/operators.cc
@@ -16,8 +16,6 @@
namespace {
-const char kSourcesName[] = "sources";
-
// Helper class used for assignment operations: =, +=, and -= to generalize
// writing to various types of destinations.
class ValueDestination {
@@ -45,10 +43,6 @@
// assumption that it will be modified in-place.
Value* GetExistingMutableValueIfExists(const ParseNode* origin);
- // Returns the sources assignment filter if it exists for the current
- // scope and it should be applied to this assignment. Otherwise returns null.
- const PatternList* GetAssignmentFilter(const Scope* exec_scope) const;
-
// Returns a pointer to the value that was set.
Value* SetValue(Value value, const ParseNode* set_node);
@@ -179,19 +173,6 @@
return nullptr;
}
-const PatternList* ValueDestination::GetAssignmentFilter(
- const Scope* exec_scope) const {
- if (type_ != SCOPE)
- return nullptr; // Destination can't be named, so no sources filtering.
- if (name_token_->value() != kSourcesName)
- return nullptr; // Destination not named "sources".
-
- const PatternList* filter = exec_scope->GetSourcesAssignmentFilter();
- if (!filter || filter->is_empty())
- return nullptr; // No filter or empty filter, don't need to do anything.
- return filter;
-}
-
Value* ValueDestination::SetValue(Value value, const ParseNode* set_node) {
if (type_ == SCOPE) {
return scope_->SetValue(name_token_->value(), std::move(value), set_node);
@@ -346,17 +327,7 @@
}
}
- Value* written_value = dest->SetValue(std::move(right), op_node->right());
-
- // Optionally apply the assignment filter in-place.
- const PatternList* filter = dest->GetAssignmentFilter(exec_scope);
- if (filter && written_value->type() == Value::LIST) {
- std::vector<Value>& list_value = written_value->list_value();
- auto first_deleted = std::remove_if(
- list_value.begin(), list_value.end(),
- [filter](const Value& v) { return filter->MatchesValue(v); });
- list_value.erase(first_deleted, list_value.end());
- }
+ dest->SetValue(std::move(right), op_node->right());
return Value();
}
@@ -501,21 +472,9 @@
} else if (mutable_dest->type() == Value::LIST) {
// List concat.
if (right.type() == Value::LIST) {
- // Note: don't reserve() the dest vector here since that actually hurts
- // the allocation pattern when the build script is doing multiple small
- // additions.
- const PatternList* filter = dest->GetAssignmentFilter(exec_scope);
- if (filter) {
- // Filtered list concat.
- for (Value& value : right.list_value()) {
- if (!filter->MatchesValue(value))
- mutable_dest->list_value().push_back(std::move(value));
- }
- } else {
- // Normal list concat. This is a destructive move.
- for (Value& value : right.list_value())
- mutable_dest->list_value().push_back(std::move(value));
- }
+ // Normal list concat. This is a destructive move.
+ for (Value& value : right.list_value())
+ mutable_dest->list_value().push_back(std::move(value));
} else {
*err = Err(op_node->op(), "Incompatible types to add.",
"To append a single item to a list do \"foo += [ bar ]\".");
diff --git a/src/gn/operators_unittest.cc b/src/gn/operators_unittest.cc
index 3c39ae2..415596c 100644
--- a/src/gn/operators_unittest.cc
+++ b/src/gn/operators_unittest.cc
@@ -109,11 +109,6 @@
TestBinaryOpNode node(Token::PLUS_EQUALS, "+=");
node.SetLeftToIdentifier(sources);
- // Set up the filter on the scope to remove everything ending with "rm"
- std::unique_ptr<PatternList> pattern_list = std::make_unique<PatternList>();
- pattern_list->Append(Pattern("*rm"));
- setup.scope()->set_sources_assignment_filter(std::move(pattern_list));
-
// Append an integer.
node.SetRightToListOfValue(Value(nullptr, static_cast<int64_t>(5)));
node.Execute(setup.scope(), &err);
@@ -125,14 +120,8 @@
node.Execute(setup.scope(), &err);
EXPECT_FALSE(err.has_error());
- // Append a string that does match the pattern, it should be a no-op.
- const char string2[] = "foo-rm";
- node.SetRightToListOfValue(Value(nullptr, string2));
- node.Execute(setup.scope(), &err);
- EXPECT_FALSE(err.has_error());
-
// Append a list with the two strings from above.
- node.SetRightToListOfValue(Value(nullptr, string1), Value(nullptr, string2));
+ node.SetRightToListOfValue(Value(nullptr, string1));
node.Execute(setup.scope(), &err);
EXPECT_FALSE(err.has_error());
diff --git a/src/gn/parser.cc b/src/gn/parser.cc
index 833e590..b8919ff 100644
--- a/src/gn/parser.cc
+++ b/src/gn/parser.cc
@@ -179,10 +179,6 @@
mylist = []
mylist = otherlist
- When assigning to a list named 'sources' using '=' or '+=', list items may be
- automatically filtered out. See "gn help set_sources_assignment_filter" for
- more.
-
Scopes
All execution happens in the context of a scope which holds the current state
diff --git a/src/gn/scope.cc b/src/gn/scope.cc
index a220c8e..a1cf179 100644
--- a/src/gn/scope.cc
+++ b/src/gn/scope.cc
@@ -379,23 +379,6 @@
"<SHOULDN'T HAPPEN>", err);
}
- // Sources assignment filter.
- if (sources_assignment_filter_) {
- if (!options.clobber_existing) {
- if (dest->GetSourcesAssignmentFilter()) {
- // Sources assignment filter present in both the source and the dest.
- std::string desc_string(desc_for_err);
- *err = Err(node_for_err, "Assignment filter collision.",
- "The " + desc_string +
- " contains a sources_assignment_filter "
- "which\nwould clobber the one in your current scope.");
- return false;
- }
- }
- dest->sources_assignment_filter_ =
- std::make_unique<PatternList>(*sources_assignment_filter_);
- }
-
// Templates.
for (const auto& pair : templates_) {
const std::string& current_name = pair.first;
@@ -483,14 +466,6 @@
return nullptr;
}
-const PatternList* Scope::GetSourcesAssignmentFilter() const {
- if (sources_assignment_filter_)
- return sources_assignment_filter_.get();
- if (containing())
- return containing()->GetSourcesAssignmentFilter();
- return nullptr;
-}
-
void Scope::SetProcessingBuildConfig() {
DCHECK((mode_flags_ & kProcessingBuildConfigFlag) == 0);
mode_flags_ |= kProcessingBuildConfigFlag;
diff --git a/src/gn/scope.h b/src/gn/scope.h
index e800621..4621db5 100644
--- a/src/gn/scope.h
+++ b/src/gn/scope.h
@@ -255,12 +255,6 @@
// been set.
const Scope* GetTargetDefaults(const std::string& target_type) const;
- // Filter to apply when the sources variable is assigned. May return NULL.
- const PatternList* GetSourcesAssignmentFilter() const;
- void set_sources_assignment_filter(std::unique_ptr<PatternList> f) {
- sources_assignment_filter_ = std::move(f);
- }
-
// Indicates if we're currently processing the build configuration file.
// This is true when processing the config file for any toolchain.
//
@@ -370,10 +364,6 @@
using NamedScopeMap = std::unordered_map<std::string, std::unique_ptr<Scope>>;
NamedScopeMap target_defaults_;
- // Null indicates not set and that we should fallback to the containing
- // scope's filter.
- std::unique_ptr<PatternList> sources_assignment_filter_;
-
// Owning pointers, must be deleted.
using TemplateMap = std::map<std::string, scoped_refptr<const Template>>;
TemplateMap templates_;