diff options
author | newt@chromium.org <newt@chromium.org@7262f16d-afe8-6277-6482-052fa10e57b1> | 2014-02-07 21:01:12 +0000 |
---|---|---|
committer | newt@chromium.org <newt@chromium.org@7262f16d-afe8-6277-6482-052fa10e57b1> | 2014-02-07 21:01:12 +0000 |
commit | 977eb64972b3d1f6e24e878fc0858cb33aef25e2 (patch) | |
tree | a01cbc3d22d37a3ee817fc3359a3c5c88d75b107 | |
parent | db58423a76618bf8ee4dc1f5b5c9ee1ba1f3d4c9 (diff) | |
download | grit-977eb64972b3d1f6e24e878fc0858cb33aef25e2.tar.gz |
Provide defines as local variables in if-expressions.
If-expressions can now access the values of variables defined at the
command line (using -D or -E). Undefined variables default to False.
This enables many if-expressions to be simplified, e.g.:
Before: <if expr="pp_ifdef('enable_foo')">
After: <if expr="enable_foo">
Before: <if expr="defs['foo'] == 'bar'">
After: <if expr="foo == 'bar'">
This also improves evaluation performance by caching compiled code
objects, leading to a 3x evaluation speedup while processing
generated_resources.grd (0.35 -> 0.12 sec on my machine).
R=joi@chromium.org
Review URL: https://codereview.chromium.org/156443002
git-svn-id: http://grit-i18n.googlecode.com/svn/trunk@150 7262f16d-afe8-6277-6482-052fa10e57b1
-rw-r--r-- | grit/format/resource_map_unittest.py | 4 | ||||
-rw-r--r-- | grit/grd_reader_unittest.py | 12 | ||||
-rw-r--r-- | grit/node/base.py | 127 | ||||
-rw-r--r-- | grit/node/base_unittest.py | 61 |
4 files changed, 130 insertions, 74 deletions
diff --git a/grit/format/resource_map_unittest.py b/grit/format/resource_map_unittest.py index 6112e03..8f162b5 100644 --- a/grit/format/resource_map_unittest.py +++ b/grit/format/resource_map_unittest.py @@ -109,13 +109,13 @@ const size_t kTheRcHeaderSize = arraysize(kTheRcHeader);''', output) <message name="IDS_PRODUCT_NAME" desc="The application name"> Application </message> - <if expr="1"> + <if expr="True"> <message name="IDS_DEFAULT_TAB_TITLE_TITLE_CASE" desc="In Title Case: The default title in a tab."> New Tab </message> </if> - <if expr="0"> + <if expr="False"> <message name="IDS_DEFAULT_TAB_TITLE" desc="The default title in a tab."> New tab diff --git a/grit/grd_reader_unittest.py b/grit/grd_reader_unittest.py index 065d280..fd4d08b 100644 --- a/grit/grd_reader_unittest.py +++ b/grit/grd_reader_unittest.py @@ -289,14 +289,6 @@ class GrdReaderUnittest(unittest.TestCase): def testEarlyEnoughPlatformSpecification(self): # This is a regression test for issue # https://code.google.com/p/grit-i18n/issues/detail?id=23 - # - # This test only works on platforms for which one of the is_xyz - # shortcuts in .grd expressions is true. If this string remains - # empty, abort the test. - platform = base.Node.GetPlatformAssertion(sys.platform) - if not platform: - return - grd_text = u'''<?xml version="1.0" encoding="UTF-8"?> <grit latest_public_release="1" current_release="1"> <release seq="1"> @@ -307,12 +299,12 @@ class GrdReaderUnittest(unittest.TestCase): <!-- The assumption is that use_titlecase is never true for this platform. When the platform isn't set to 'android' early enough, we get a duplicate message name. --> - <if expr="%s"> + <if expr="os == '%s'"> <message name="IDS_XYZ">boo</message> </if> </messages> </release> - </grit>''' % platform + </grit>''' % sys.platform with util.TempDir({}) as temp_dir: grd_reader.Parse(StringIO.StringIO(grd_text), temp_dir.GetPath(), target_platform='android') diff --git a/grit/node/base.py b/grit/node/base.py index 824532c..5ce037f 100644 --- a/grit/node/base.py +++ b/grit/node/base.py @@ -6,9 +6,8 @@ '''Base types for nodes in a GRIT resource tree. ''' -import collections +import ast import os -import sys import types from xml.sax import saxutils @@ -28,11 +27,12 @@ class Node(object): # Default nodes to not whitelist skipped _whitelist_marked_as_skip = False - # A class-static cache to memoize EvaluateExpression(). - # It has a 2 level nested dict structure. The outer dict has keys - # of tuples which define the environment in which the expression - # will be evaluated. The inner dict is map of expr->result. - eval_expr_cache = collections.defaultdict(dict) + # A class-static cache to speed up EvaluateExpression(). + # Keys are expressions (e.g. 'is_ios and lang == "fr"'). Values are tuples + # (code, variables_in_expr) where code is the compiled expression and can be + # directly eval'd, and variables_in_expr is the list of variable and method + # names used in the expression (e.g. ['is_ios', 'lang']). + eval_expr_cache = {} def __init__(self): self.children = [] # A list of child elements @@ -442,59 +442,62 @@ class Node(object): return [] @classmethod - def GetPlatformAssertion(cls, target_platform): - '''If the platform is a specific well-known platform, this returns - the is_xyz string representing that platform (e.g. is_linux), - otherwise the empty string. - ''' - platform = '' - if target_platform == 'darwin': - platform = 'is_macosx' - elif target_platform.startswith('linux'): - platform = 'is_linux' - elif target_platform in ('cygwin', 'win32'): - platform = 'is_win' - elif target_platform in ('android', 'ios'): - platform = 'is_%s' % target_platform - return platform - - @classmethod - def EvaluateExpression(cls, expr, defs, target_platform, extra_variables=None): + def EvaluateExpression(cls, expr, defs, target_platform, extra_variables={}): '''Worker for EvaluateCondition (below) and conditions in XTB files.''' - cache_dict = cls.eval_expr_cache[ - (tuple(defs.iteritems()), target_platform, extra_variables)] - if expr in cache_dict: - return cache_dict[expr] - def pp_ifdef(symbol): - return symbol in defs - def pp_if(symbol): - return defs.get(symbol, False) - variable_map = { - 'defs' : defs, - 'os': target_platform, - - # One of these is_xyz assertions gets set to True in the line - # following this initializer block. - 'is_linux': False, - 'is_macosx': False, - 'is_win': False, - 'is_android': False, - 'is_ios': False, - - # is_posix is not mutually exclusive of the others and gets - # set here, not below. - 'is_posix': (target_platform in ('darwin', 'linux2', 'linux3', 'sunos5', - 'android', 'ios') - or 'bsd' in target_platform), - - 'pp_ifdef' : pp_ifdef, - 'pp_if' : pp_if, - } - variable_map[Node.GetPlatformAssertion(target_platform)] = True + if expr in cls.eval_expr_cache: + code, variables_in_expr = cls.eval_expr_cache[expr] + else: + # Get a list of all variable and method names used in the expression. + syntax_tree = ast.parse(expr, mode='eval') + variables_in_expr = [node.id for node in ast.walk(syntax_tree) if + isinstance(node, ast.Name) and node.id not in ('True', 'False')] + code = compile(syntax_tree, filename='<string>', mode='eval') + cls.eval_expr_cache[expr] = code, variables_in_expr + + # Set values only for variables that are needed to eval the expression. + variable_map = {} + for name in variables_in_expr: + if name == 'os': + value = target_platform + elif name == 'defs': + value = defs + + elif name == 'is_linux': + value = target_platform.startswith('linux') + elif name == 'is_macosx': + value = target_platform == 'darwin' + elif name == 'is_win': + value = target_platform in ('cygwin', 'win32') + elif name == 'is_android': + value = target_platform == 'android' + elif name == 'is_ios': + value = target_platform == 'ios' + elif name == 'is_posix': + value = (target_platform in ('darwin', 'linux2', 'linux3', 'sunos5', + 'android', 'ios') + or 'bsd' in target_platform) + + elif name == 'pp_ifdef': + def pp_ifdef(symbol): + return symbol in defs + value = pp_ifdef + elif name == 'pp_if': + def pp_if(symbol): + return defs.get(symbol, False) + value = pp_if + + elif name in defs: + value = defs[name] + elif name in extra_variables: + value = extra_variables[name] + else: + # Undefined variables default to False. + value = False - if extra_variables: - variable_map.update(extra_variables) - eval_result = cache_dict[expr] = eval(expr, {}, variable_map) + variable_map[name] = value + + eval_result = eval(code, {}, variable_map) + assert isinstance(eval_result, bool) return eval_result def EvaluateCondition(self, expr): @@ -518,10 +521,10 @@ class Node(object): context = getattr(root, 'output_context', '') defs = getattr(root, 'defines', {}) target_platform = getattr(root, 'target_platform', '') - extra_variables = ( - ('lang', lang), - ('context', context), - ) + extra_variables = { + 'lang': lang, + 'context': context, + } return Node.EvaluateExpression( expr, defs, target_platform, extra_variables) diff --git a/grit/node/base_unittest.py b/grit/node/base_unittest.py index 63a2033..f7e7b75 100644 --- a/grit/node/base_unittest.py +++ b/grit/node/base_unittest.py @@ -192,6 +192,67 @@ class NodeUnittest(unittest.TestCase): self.failUnlessEqual(output_nodes[2].attrs['filename'], 'de/generated_resources.rc') + def testEvaluateExpression(self): + def AssertExpr(expected_value, expr, defs, target_platform, + extra_variables): + self.failUnlessEqual(expected_value, base.Node.EvaluateExpression( + expr, defs, target_platform, extra_variables)) + + AssertExpr(True, "True", {}, 'linux', {}) + AssertExpr(False, "False", {}, 'linux', {}) + AssertExpr(True, "True or False", {}, 'linux', {}) + AssertExpr(False, "True and False", {}, 'linux', {}) + AssertExpr(True, "os == 'linux'", {}, 'linux', {}) + AssertExpr(False, "os == 'linux'", {}, 'ios', {}) + AssertExpr(True, "'foo' in defs", {'foo': 'bar'}, 'ios', {}) + AssertExpr(False, "'foo' in defs", {'baz': 'bar'}, 'ios', {}) + AssertExpr(False, "'foo' in defs", {}, 'ios', {}) + AssertExpr(True, "is_linux", {}, 'linux2', {}) + AssertExpr(False, "is_linux", {}, 'win32', {}) + AssertExpr(True, "is_macosx", {}, 'darwin', {}) + AssertExpr(False, "is_macosx", {}, 'ios', {}) + AssertExpr(True, "is_win", {}, 'win32', {}) + AssertExpr(False, "is_win", {}, 'darwin', {}) + AssertExpr(True, "is_android", {}, 'android', {}) + AssertExpr(False, "is_android", {}, 'linux3', {}) + AssertExpr(True, "is_ios", {}, 'ios', {}) + AssertExpr(False, "is_ios", {}, 'darwin', {}) + AssertExpr(True, "is_posix", {}, 'linux2', {}) + AssertExpr(True, "is_posix", {}, 'darwin', {}) + AssertExpr(True, "is_posix", {}, 'android', {}) + AssertExpr(True, "is_posix", {}, 'ios', {}) + AssertExpr(True, "is_posix", {}, 'freebsd7', {}) + AssertExpr(False, "is_posix", {}, 'win32', {}) + AssertExpr(True, "pp_ifdef('foo')", {'foo': True}, 'win32', {}) + AssertExpr(True, "pp_ifdef('foo')", {'foo': False}, 'win32', {}) + AssertExpr(False, "pp_ifdef('foo')", {'bar': True}, 'win32', {}) + AssertExpr(True, "pp_if('foo')", {'foo': True}, 'win32', {}) + AssertExpr(False, "pp_if('foo')", {'foo': False}, 'win32', {}) + AssertExpr(False, "pp_if('foo')", {'bar': True}, 'win32', {}) + AssertExpr(True, "foo", {'foo': True}, 'win32', {}) + AssertExpr(False, "foo", {'foo': False}, 'win32', {}) + AssertExpr(False, "foo", {'bar': True}, 'win32', {}) + AssertExpr(True, "foo == 'baz'", {'foo': 'baz'}, 'win32', {}) + AssertExpr(False, "foo == 'baz'", {'foo': True}, 'win32', {}) + AssertExpr(False, "foo == 'baz'", {}, 'win32', {}) + AssertExpr(True, "lang == 'de'", {}, 'win32', {'lang': 'de'}) + AssertExpr(False, "lang == 'de'", {}, 'win32', {'lang': 'fr'}) + AssertExpr(False, "lang == 'de'", {}, 'win32', {}) + + # Test a couple more complex expressions for good measure. + AssertExpr(True, "is_ios and (lang in ['de', 'fr'] or foo)", + {'foo': 'bar'}, 'ios', {'lang': 'fr', 'context': 'today'}) + AssertExpr(False, "is_ios and (lang in ['de', 'fr'] or foo)", + {'foo': False}, 'linux2', {'lang': 'fr', 'context': 'today'}) + AssertExpr(False, "is_ios and (lang in ['de', 'fr'] or foo)", + {'baz': 'bar'}, 'ios', {'lang': 'he', 'context': 'today'}) + AssertExpr(True, "foo == 'bar' or not baz", + {'foo': 'bar', 'fun': True}, 'ios', {'lang': 'en'}) + AssertExpr(True, "foo == 'bar' or not baz", + {}, 'ios', {'lang': 'en', 'context': 'java'}) + AssertExpr(False, "foo == 'bar' or not baz", + {'foo': 'ruz', 'baz': True}, 'ios', {'lang': 'en'}) + if __name__ == '__main__': unittest.main() |