Refactor GN source expansions.

Refactor GN source expansions.

This is in preparation for using {{patterns}} in toolchain definitions. It makes them
more generic and usable for this, without actually implementing any new features or
changing behavior. The only external change should be that the $in variable is used for
actions instead of making a redundant new variable (used to be called $source).

It removes the FileTemplate class and explodes it into several files: the type list and
helpers (substitution_type), a single pattern (substitution_pattern), a list of patterns
(substitution_list), and a class of helper functions for processing templates in various
ways (substitution_writer).

Previously, the things needing substitutions (args, outputs, depfile) were stored as
strings and parsed in to FileTemplates as needed. This new method stores the
SubstitutionList/Pattern directly on the target. This allows it to issue parse errors
(previously such errors were ignored since errors weren't reportable when writing Ninja
files) and cleans up a lot of the code that uses it.

R=scottmg@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287847

[scottmg, 2014-08-05 17:13:29]

Sorry for the delay. Holiday yesterday, moving this morning. I'll look at it
today.

[scottmg, 2014-08-05 22:30:33]

lgtm

https://codereview.chromium.org/429423002/diff/80001/tools/gn/command_help.cc
File tools/gn/command_help.cc (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/command_help.cc...
tools/gn/command_help.cc:16: #include "tools/gn/substitution_writer.h"
necessary?

https://codereview.chromium.org/429423002/diff/80001/tools/gn/function_get_ta...
File tools/gn/function_get_target_outputs.cc (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/function_get_ta...
tools/gn/function_get_target_outputs.cc:29: settings,
target->action_values().outputs(), sources, ret);
What will happen if there is a substitution? Maybe a test for this case.

https://codereview.chromium.org/429423002/diff/80001/tools/gn/source_file.h
File tools/gn/source_file.h (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/source_file.h#n...
tools/gn/source_file.h:30: explicit SourceFile(SwapIn, std::string* value);
no explicit necessary

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_li...
File tools/gn/substitution_list.h (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_li...
tools/gn/substitution_list.h:25: static SubstitutionList MakeForTest(
Seems like this doesn't need to be in-class.

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_pa...
File tools/gn/substitution_pattern.cc (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_pa...
tools/gn/substitution_pattern.cc:71: // Expect all occurrances of {{ to resolve
to a pattern.
nit; "occurrences"

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_pa...
tools/gn/substitution_pattern.cc:87: memset(&required_type_bits, 0,
SUBSTITUTION_NUM_TYPES);
sizeof bool != 1 => sadness. "bool required_type_bits[N] = {0};" would be
better.

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_pa...
File tools/gn/substitution_pattern_unittest.cc (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_pa...
tools/gn/substitution_pattern_unittest.cc:43:
EXPECT_FALSE(pattern.Parse("{{source_of_evil}}", NULL, &err));
Will {{source{{source}}}} fail properly too? I wasn't quite sure without trying
it.

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_wr...
File tools/gn/substitution_writer.cc (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_wr...
tools/gn/substitution_writer.cc:261: /*
What's up here?

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_wr...
File tools/gn/substitution_writer_unittest.cc (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_wr...
tools/gn/substitution_writer_unittest.cc:149: 
The other SUBSTITUTION_ types will be exercised once it's used for toolchains?

[brettw, 2014-08-06 18:25:01]

https://codereview.chromium.org/429423002/diff/80001/tools/gn/command_help.cc
File tools/gn/command_help.cc (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/command_help.cc...
tools/gn/command_help.cc:16: #include "tools/gn/substitution_writer.h"
On 2014/08/05 22:30:32, scottmg wrote:
> necessary?

Yeah, the help text for substitutions is in that file.

https://codereview.chromium.org/429423002/diff/80001/tools/gn/function_get_ta...
File tools/gn/function_get_target_outputs.cc (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/function_get_ta...
tools/gn/function_get_target_outputs.cc:29: settings,
target->action_values().outputs(), sources, ret);
On 2014/08/05 22:30:32, scottmg wrote:
> What will happen if there is a substitution? Maybe a test for this case.

The thing that makes the target should prohibit this. I fixed this and added a
test.

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_li...
File tools/gn/substitution_list.h (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_li...
tools/gn/substitution_list.h:25: static SubstitutionList MakeForTest(
On 2014/08/05 22:30:32, scottmg wrote:
> Seems like this doesn't need to be in-class.

No, but it seems like the best place for it to me, unless I were to add some
kind of separate "substitution test util" file which seemed unnecessary for this
small thing.

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_pa...
File tools/gn/substitution_pattern_unittest.cc (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_pa...
tools/gn/substitution_pattern_unittest.cc:43:
EXPECT_FALSE(pattern.Parse("{{source_of_evil}}", NULL, &err));
On 2014/08/05 22:30:32, scottmg wrote:
> Will {{source{{source}}}} fail properly too? I wasn't quite sure without
trying
> it.

Yes, I added this case to the test.

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_wr...
File tools/gn/substitution_writer_unittest.cc (right):

https://codereview.chromium.org/429423002/diff/80001/tools/gn/substitution_wr...
tools/gn/substitution_writer_unittest.cc:149: 
On 2014/08/05 22:30:32, scottmg wrote:
> The other SUBSTITUTION_ types will be exercised once it's used for toolchains?

Yes

[brettw, 2014-08-06 20:38:51]

Committed patchset #6 manually as 287847 (presubmit successful).