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_;