0

Allow multiple conditions in mojom's EnableIf attribute

Before this CL mojom EnableIf attributes could only test a single
feature per definition, requiring work in build.gn files to allow
for multiple-os enablement:

```mojom
[EnableIf=is_chromeos_or_linux]
struct Foo {
  ...
}
```

```gn
mojom("foo") {
    ...
    if (is_chromeos | is_linux) {
        enabled_features += "is_chromeos_or_linux"
    }
}
```

After this CL a pipe `|` or ampersand `&` can now be used to
allow multiple conditions to be tested for `EnableIf` and
`EnableIfNot` attributes. If `|` is used then any listed
feature being enabled will make the condition true (so an
EnableIf attribute will allow the mojom node to be defined).
If `&` is supplied every listed feature must be present in
enabled_features for the condition to be true. This will
allow simplification of the build setup for many mojoms where
features are introduced to allow multiple platforms to be
specified:

```mojom
[EnableIf=is_chromeos|is_linux]
struct Foo {
  ...
}
```

This change is backwards-compatible as previously the `|`
and `&` characters could not be included in an attribute's
value field and would result in a parse error.

No other attributes make use of these facilities, and if `&`
or `|` are provided this will likely result in a python error
during mojom generation. Mixed `&` and `|` are prevented by
the parsing rules.

Tests: mojo/public/tools/run_all_python_unittests.py
Bug: 378692747
Change-Id: I4873a3ced25e60796a63889153ae8ef457c2465b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6014778
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1382566}
This commit is contained in:
Alex Gough
2024-11-13 20:59:09 +00:00
committed by Chromium LUCI CQ
parent 11e78cd74a
commit 899cca9d77
6 changed files with 149 additions and 20 deletions

@@ -512,11 +512,13 @@ interesting attributes supported today.
matching `value` in the list of `enabled_features`, the definition will be matching `value` in the list of `enabled_features`, the definition will be
disabled. This is useful for mojom definitions that only make sense on one disabled. This is useful for mojom definitions that only make sense on one
platform. Note that the `EnableIf` attribute can only be set once per platform. Note that the `EnableIf` attribute can only be set once per
definition and cannot be set at the same time as `EnableIfNot`. Also be aware definition and cannot be set at the same time as `EnableIfNot`. Multiple
that only one condition can be tested, `EnableIf=value,xyz` introduces a new conditions can be tested using `|` (any, e.g. `EnableIf=is_win|is_linux`) and
`xyz` attribute. `xyz` is not part of the `EnableIf` condition that depends `&` (all, e.g. `Enableif=is_official_build&is_win`). You cannot mix `&` and
only on the feature `value`. Complex conditions can be introduced via `|` in one condition. More complex conditions can be introduced by defining
enabled_features in `build.gn` files. your own features via `enabled_features` in `build.gn` files. Also be aware
that a comma introduces a new attribute, so `EnableIf=value,xyz` means
`EnableIf=value` and applies the `xyz` attribute.
* **`[EnableIfNot=value]`**: * **`[EnableIfNot=value]`**:
The `EnableIfNot` attribute is used to conditionally enable definitions when The `EnableIfNot` attribute is used to conditionally enable definitions when
@@ -524,7 +526,10 @@ interesting attributes supported today.
matching `value` in the list of `enabled_features`, the definition will be matching `value` in the list of `enabled_features`, the definition will be
disabled. This is useful for mojom definitions that only make sense on all but disabled. This is useful for mojom definitions that only make sense on all but
one platform. Note that the `EnableIfNot` attribute can only be set once per one platform. Note that the `EnableIfNot` attribute can only be set once per
definition and cannot be set at the same time as `EnableIf`. definition and cannot be set at the same time as `EnableIf`. Multiple
conditions can be tested using `|` (any, e.g. `EnableIfNot=is_win|is_linux`)
and `&` (all, e.g. `EnableifNot=is_official_build&is_win`). You cannot mix `&`
and `|` in one condition.
* **`[ServiceSandbox=value]`**: * **`[ServiceSandbox=value]`**:
The `ServiceSandbox` attribute is used in Chromium to tag which sandbox a The `ServiceSandbox` attribute is used in Chromium to tag which sandbox a

@@ -203,11 +203,29 @@ class Attribute(NodeBase):
class AttributeList(NodeListBase): class AttributeList(NodeListBase):
"""Represents a list attributes.""" """Represents a list of attributes."""
_list_item_type = Attribute _list_item_type = Attribute
class AttributeValueOrList(NodeListBase):
"""Represents a list of pipe delimited attribute values."""
def __str__(self):
return '|'.join(item.name for item in self)
_list_item_type = Name
class AttributeValueAndList(NodeListBase):
"""Represents a list of ampersand delimited attribute values."""
def __str__(self):
return '&'.join(item.name for item in self)
_list_item_type = Name
class Const(Definition): class Const(Definition):
"""Represents a const definition.""" """Represents a const definition."""
@@ -226,6 +244,11 @@ class Const(Definition):
self.typename == other.typename and \ self.typename == other.typename and \
self.value == other.value self.value == other.value
def __repr__(self):
return "Const(typename=%s, attribute_list=%s, value=%s)" % (
self.typename, self.attribute_list, self.value)
class Enum(Definition): class Enum(Definition):
"""Represents an enum definition.""" """Represents an enum definition."""

@@ -27,24 +27,52 @@ def _IsEnabled(definition, enabled_features):
if not definition.attribute_list: if not definition.attribute_list:
return True return True
already_defined = False has_condition = False
for a in definition.attribute_list: condition = None # EnableIf or EnableIfNot
if a.key.name == 'EnableIf' or a.key.name == 'EnableIfNot': value = None
if already_defined: for attribute in definition.attribute_list:
if attribute.key.name == 'EnableIf' or attribute.key.name == 'EnableIfNot':
if has_condition:
raise EnableIfError( raise EnableIfError(
definition.filename, definition.filename,
"EnableIf/EnableIfNot attribute may only be set once per field.", "EnableIf/EnableIfNot attribute may only be set once per field.",
definition.start.line) definition.start.line)
already_defined = True condition = attribute.key.name
value = attribute.value
has_condition = True
for attribute in definition.attribute_list: # No EnableIf/EnableIfNot to filter by, so item is defined.
if (attribute.key.name == 'EnableIf' if not has_condition:
and attribute.value.name not in enabled_features): return True
return False
if (attribute.key.name == 'EnableIfNot' # Common case is to have a single attribute value so shortcut that:
and attribute.value.name in enabled_features): if not isinstance(value, ast.NodeListBase):
return False if condition == 'EnableIf':
return True if value.name not in enabled_features:
return False
if condition == 'EnableIfNot':
if value.name in enabled_features:
return False
return True
condition_met = False
if isinstance(value, ast.AttributeValueOrList):
for item in value:
if item.name in enabled_features:
condition_met = True
break
elif isinstance(value, ast.AttributeValueAndList):
for item in value:
if item.name in enabled_features:
condition_met = True
continue
condition_met = False
break
if condition == 'EnableIf':
return condition_met
return not condition_met
def _FilterDisabledFromNodeList(node_list, enabled_features): def _FilterDisabledFromNodeList(node_list, enabled_features):

@@ -372,5 +372,54 @@ class ConditionalFeaturesTest(unittest.TestCase):
conditional_features.RemoveDisabledDefinitions, conditional_features.RemoveDisabledDefinitions,
definition, ENABLED_FEATURES) definition, ENABLED_FEATURES)
def testMultipleOrFeatures(self):
mojom_source = """
feature Foo {
const string name = "FooFeature";
[EnableIf=red|yellow]
const bool default_state = false;
[EnableIf=yellow|purple]
const bool default_state = true;
};
"""
expected_source = """
feature Foo {
const string name = "FooFeature";
[EnableIf=red|yellow]
const bool default_state = false;
};
"""
self.parseAndAssertEqual(mojom_source, expected_source)
def testMultipleAndFeatures(self):
mojom_source = """
feature Foo {
const string name = "FooFeature";
[EnableIf=red&blue]
const bool default_state = false;
[EnableIf=yellow&purple]
const bool default_state = true;
};
"""
expected_source = """
feature Foo {
const string name = "FooFeature";
[EnableIf=red&blue]
const bool default_state = false;
};
"""
self.parseAndAssertEqual(mojom_source, expected_source)
def testMixedAndOrInEnableIf(self):
source = """
enum Foo {
[EnableIf=red&blue|yellow]
kBarValue = 5,
};
"""
# some other error, but some error!
self.assertRaises(parser.ParseError, parser.Parse, source, "myfile.mojom")
if __name__ == '__main__': if __name__ == '__main__':
unittest.main() unittest.main()

@@ -101,6 +101,8 @@ class Lexer:
'RANGLE', # < > 'RANGLE', # < >
'SEMI', # ; 'SEMI', # ;
'COMMA', 'COMMA',
'PIPE', # |
'AMPERSAND', # &
'DOT' # , . 'DOT' # , .
) )
@@ -190,6 +192,8 @@ class Lexer:
t_COMMA = r',' t_COMMA = r','
t_DOT = r'\.' t_DOT = r'\.'
t_SEMI = r';' t_SEMI = r';'
t_PIPE = r'\|'
t_AMPERSAND = r'&'
t_STRING_LITERAL = string_literal t_STRING_LITERAL = string_literal

@@ -174,6 +174,8 @@ class Parser:
def p_attribute_2(self, p): def p_attribute_2(self, p):
"""attribute : name EQUALS evaled_literal """attribute : name EQUALS evaled_literal
| name EQUALS nonempty_pipe_delimited_names
| name EQUALS nonempty_amps_delimited_names
| name EQUALS name""" | name EQUALS name"""
p[0] = ast.Attribute(p[1], p[3]) p[0] = ast.Attribute(p[1], p[3])
self._set_lexstate(p, 1, 3) self._set_lexstate(p, 1, 3)
@@ -195,6 +197,24 @@ class Parser:
else: else:
p[0] = eval(p[1].value) p[0] = eval(p[1].value)
def p_nonempty_pipe_delimited_names_1(self, p):
"""nonempty_pipe_delimited_names : name"""
p[0] = ast.AttributeValueOrList(p[1])
def p_nonempty_pipe_delimited_names_2(self, p):
"""nonempty_pipe_delimited_names : nonempty_pipe_delimited_names PIPE name"""
p[0] = p[1]
p[0].Append(p[3])
def p_nonempty_amps_delimited_names_1(self, p):
"""nonempty_amps_delimited_names : name"""
p[0] = ast.AttributeValueAndList(p[1])
def p_nonempty_amps_delimited_names_2(self, p):
"""nonempty_amps_delimited_names : nonempty_amps_delimited_names AMPERSAND name"""
p[0] = p[1]
p[0].Append(p[3])
def p_struct_1(self, p): def p_struct_1(self, p):
"""struct : attribute_section STRUCT name LBRACE struct_body RBRACE SEMI""" """struct : attribute_section STRUCT name LBRACE struct_body RBRACE SEMI"""
p[0] = ast.Struct(p[3], p[1], p[5]) p[0] = ast.Struct(p[3], p[1], p[5])