Factor back-end attribute checking into the back end. (#80)
Factor back-end attribute checking into the back end.
This change splits up `front_end/attribute_checker.py`, moving the
generic parts to a new file `util/attribute_util.py`, and moving the
back-end-specific parts to `back_end/cpp/header_generator.py`.
Some tests from `front_end/attribute_checker_test.py` were moved to
a new test suite, `header_generator_test.py`. There should probably
be a `util/attribute_checker_test.py`, but for now the old tests
provide sufficient coverage.
As a result of moving some attribute checking into the back end,
`generate_header()` can now return errors. A future change should
convert some of the `assert` statements in the same file into error
returns.
In order for the `emboss_codegen_cpp.py` driver to properly display
errors, the original source code of the `.emb` is now included
verbatim in the IR, increasing the IR size by about 3%.
This change does still enforce some minimal checking of back end
attributes: the back end must be listed in the new
`[expected_back_ends]` attribute (default value `"cpp"`), or it is
considered to be an error. This change does not document the
`[expected_back_ends]` attribute because it is not currently useful for
end users.
diff --git a/compiler/back_end/cpp/BUILD b/compiler/back_end/cpp/BUILD
index c9e879c..2280c36 100644
--- a/compiler/back_end/cpp/BUILD
+++ b/compiler/back_end/cpp/BUILD
@@ -42,12 +42,22 @@
],
deps = [
"//compiler/back_end/util:code_template",
+ "//compiler/util:attribute_util",
"//compiler/util:ir_pb2",
"//compiler/util:ir_util",
"//compiler/util:name_conversion",
],
)
+py_test(
+ name = "header_generator_test",
+ srcs = ["header_generator_test.py"],
+ deps = [
+ ":header_generator",
+ "//compiler/front_end:glue",
+ ],
+)
+
emboss_cc_test(
name = "span_se_log_file_status_emb_generated_code_test",
srcs = [
diff --git a/compiler/back_end/cpp/emboss_codegen_cpp.py b/compiler/back_end/cpp/emboss_codegen_cpp.py
index 73a1450..77bcb84 100644
--- a/compiler/back_end/cpp/emboss_codegen_cpp.py
+++ b/compiler/back_end/cpp/emboss_codegen_cpp.py
@@ -21,9 +21,11 @@
from __future__ import print_function
import argparse
+import os
import sys
from compiler.back_end.cpp import header_generator
+from compiler.util import error
from compiler.util import ir_pb2
@@ -38,16 +40,35 @@
type=str,
help="Write header to file. If not specified, write " +
"header to stdout.")
+ parser.add_argument("--color-output",
+ default="if_tty",
+ choices=["always", "never", "if_tty", "auto"],
+ help="Print error messages using color. 'auto' is a "
+ "synonym for 'if_tty'.")
return parser.parse_args(argv[1:])
+def _show_errors(errors, ir, flags):
+ """Prints errors with source code snippets."""
+ source_codes = {}
+ for module in ir.module:
+ source_codes[module.source_file_name] = module.source_text
+ use_color = (flags.color_output == "always" or
+ (flags.color_output in ("auto", "if_tty") and
+ os.isatty(sys.stderr.fileno())))
+ print(error.format_errors(errors, source_codes, use_color), file=sys.stderr)
+
+
def main(flags):
if flags.input_file:
with open(flags.input_file) as f:
ir = ir_pb2.EmbossIr.from_json(f.read())
else:
ir = ir_pb2.EmbossIr.from_json(sys.stdin.read())
- header = header_generator.generate_header(ir)
+ header, errors = header_generator.generate_header(ir)
+ if errors:
+ _show_errors(errors, ir, flags)
+ return 1
if flags.output_file:
with open(flags.output_file, "w") as f:
f.write(header)
@@ -56,5 +77,5 @@
return 0
-if __name__ == "__main__":
+if __name__ == '__main__':
sys.exit(main(_parse_command_line(sys.argv)))
diff --git a/compiler/back_end/cpp/header_generator.py b/compiler/back_end/cpp/header_generator.py
index 4fc7890..e467284 100644
--- a/compiler/back_end/cpp/header_generator.py
+++ b/compiler/back_end/cpp/header_generator.py
@@ -23,6 +23,7 @@
import re
from compiler.back_end.util import code_template
+from compiler.util import attribute_util
from compiler.util import ir_pb2
from compiler.util import ir_util
from compiler.util import name_conversion
@@ -1309,9 +1310,18 @@
ir: An EmbossIr of the module.
Returns:
- A string containing the text of a C++ header which implements Views for the
- types in the Emboss module.
+ A tuple of (header, errors), where `header` is either a string containing
+ the text of a C++ header which implements Views for the types in the Emboss
+ module, or None, and `errors` is a possibly-empty list of error messages to
+ display to the user.
"""
+ errors = attribute_util.check_attributes_in_ir(
+ ir,
+ back_end="cpp",
+ types={"namespace": attribute_util.STRING},
+ module_attributes={("namespace", False)})
+ if errors:
+ return None, errors
type_declarations = []
type_definitions = []
method_definitions = []
@@ -1332,4 +1342,4 @@
_TEMPLATES.outline,
includes=includes,
body=body,
- header_guard=_generate_header_guard(ir.module[0].source_file_name))
+ header_guard=_generate_header_guard(ir.module[0].source_file_name)), []
diff --git a/compiler/back_end/cpp/header_generator_test.py b/compiler/back_end/cpp/header_generator_test.py
new file mode 100644
index 0000000..e057c0d
--- /dev/null
+++ b/compiler/back_end/cpp/header_generator_test.py
@@ -0,0 +1,53 @@
+# Copyright 2023 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Tests for attribute_checker.py."""
+
+import unittest
+from compiler.back_end.cpp import header_generator
+from compiler.front_end import glue
+from compiler.util import error
+from compiler.util import ir_pb2
+from compiler.util import ir_util
+
+
+def _make_ir_from_emb(emb_text, name="m.emb"):
+ ir, unused_debug_info, errors = glue.parse_emboss_file(
+ name,
+ test_util.dict_file_reader({name: emb_text}))
+ assert not errors
+ return ir
+
+
+class NormalizeIrTest(unittest.TestCase):
+
+ def test_accepts_string_attribute(self):
+ ir = _make_ir_from_emb('[(cpp) namespace: "foo"]\n')
+ self.assertEqual([], header_generator.generate_header(ir)[1])
+
+ def test_rejects_wrong_type_for_string_attribute(self):
+ ir = _make_ir_from_emb("[(cpp) namespace: 9]\n")
+ attr = ir.module[0].attribute[0]
+ self.assertEqual([[
+ error.error("m.emb", attr.value.source_location,
+ "Attribute '(cpp) namespace' must have a string value.")
+ ]], header_generator.generate_header(ir)[1])
+
+ def test_rejects_emboss_internal_attribute_with_back_end_specifier(self):
+ ir = _make_ir_from_emb('[(cpp) byte_order: "LittleEndian"]\n')
+ attr = ir.module[0].attribute[0]
+ self.assertEqual([[
+ error.error("m.emb", attr.name.source_location,
+ "Unknown attribute '(cpp) byte_order' on module 'm.emb'.")
+ ]], attribute_checker.normalize_and_verify(ir))
diff --git a/compiler/front_end/BUILD b/compiler/front_end/BUILD
index 14a4696..f8d8484 100644
--- a/compiler/front_end/BUILD
+++ b/compiler/front_end/BUILD
@@ -213,7 +213,6 @@
name = "symbol_resolver_test",
srcs = ["symbol_resolver_test.py"],
python_version = "PY3",
- shard_count = 8,
deps = [
":glue",
":symbol_resolver",
@@ -239,7 +238,6 @@
name = "write_inference_test",
srcs = ["write_inference_test.py"],
python_version = "PY3",
- shard_count = 8,
deps = [
":glue",
":test_util",
@@ -254,8 +252,9 @@
deps = [
":attributes",
":type_check",
- "//compiler/util:ir_pb2",
+ "//compiler/util:attribute_util",
"//compiler/util:error",
+ "//compiler/util:ir_pb2",
"//compiler/util:ir_util",
"//compiler/util:traverse_ir",
],
@@ -272,7 +271,6 @@
timeout = "long",
srcs = ["attribute_checker_test.py"],
python_version = "PY3",
- shard_count = 16,
deps = [
":attribute_checker",
":glue",
@@ -299,7 +297,6 @@
name = "type_check_test",
srcs = ["type_check_test.py"],
python_version = "PY3",
- shard_count = 8,
deps = [
":glue",
":test_util",
@@ -326,7 +323,6 @@
name = "expression_bounds_test",
srcs = ["expression_bounds_test.py"],
python_version = "PY3",
- shard_count = 4,
deps = [
":expression_bounds",
":glue",
@@ -353,7 +349,6 @@
name = "constraints_test",
srcs = ["constraints_test.py"],
python_version = "PY3",
- shard_count = 8,
deps = [
":constraints",
":glue",
@@ -377,7 +372,6 @@
name = "dependency_checker_test",
srcs = ["dependency_checker_test.py"],
python_version = "PY3",
- shard_count = 8,
deps = [
":dependency_checker",
":glue",
diff --git a/compiler/front_end/attribute_checker.py b/compiler/front_end/attribute_checker.py
index 27e6722..21c6b22 100644
--- a/compiler/front_end/attribute_checker.py
+++ b/compiler/front_end/attribute_checker.py
@@ -18,143 +18,88 @@
verifies attributes which may have been manually entered.
"""
+import re
+
from compiler.front_end import attributes
from compiler.front_end import type_check
+from compiler.util import attribute_util
from compiler.util import error
from compiler.util import ir_pb2
from compiler.util import ir_util
from compiler.util import traverse_ir
-# The "namespace" attribute is C++-back-end specific, and so should not be used
-# by the front end.
-_NAMESPACE = "namespace"
-
-
-# Error messages used by multiple attribute type checkers.
-_BAD_TYPE_MESSAGE = "Attribute '{name}' must have {type} value."
-_MUST_BE_CONSTANT_MESSAGE = "Attribute '{name}' must have a constant value."
-
# Default value for maximum_bits on an `enum`.
_DEFAULT_ENUM_MAXIMUM_BITS = 64
+# Default value for expected_back_ends -- mostly for legacy
+_DEFAULT_BACK_ENDS = "cpp"
# Attribute type checkers
-def _is_constant_boolean(attr, module_source_file):
- """Checks if the given attr is a constant boolean."""
- if not attr.value.expression.type.boolean.HasField("value"):
- return [[error.error(module_source_file,
- attr.value.source_location,
- _BAD_TYPE_MESSAGE.format(name=attr.name.text,
- type="a constant boolean"))]]
+_VALID_BYTE_ORDER = attribute_util.string_from_list(
+ {"BigEndian", "LittleEndian", "Null"})
+_VALID_TEXT_OUTPUT = attribute_util.string_from_list({"Emit", "Skip"})
+
+
+def _valid_back_ends(attr, module_source_file):
+ if not re.match(
+ r"^(?:\s*[a-z][a-z0-9_]*\s*(?:,\s*[a-z][a-z0-9_]*\s*)*,?)?\s*$",
+ attr.value.string_constant.text):
+ return [[error.error(
+ module_source_file,
+ attr.value.source_location,
+ "Attribute '{name}' must be a comma-delimited list of back end "
+ "specifiers (like \"cpp, proto\")), not \"{value}\".".format(
+ name=attr.name.text,
+ value=attr.value.string_constant.text))]]
return []
-def _is_boolean(attr, module_source_file):
- """Checks if the given attr is a boolean."""
- if attr.value.expression.type.WhichOneof("type") != "boolean":
- return [[error.error(module_source_file,
- attr.value.source_location,
- _BAD_TYPE_MESSAGE.format(name=attr.name.text,
- type="a boolean"))]]
- return []
-
-
-def _is_constant_integer(attr, module_source_file):
- """Checks if the given attr is an integer constant expression."""
- if (not attr.value.HasField("expression") or
- attr.value.expression.type.WhichOneof("type") != "integer"):
- return [[error.error(module_source_file,
- attr.value.source_location,
- _BAD_TYPE_MESSAGE.format(name=attr.name.text,
- type="an integer"))]]
- if not ir_util.is_constant(attr.value.expression):
- return [[error.error(module_source_file,
- attr.value.source_location,
- _MUST_BE_CONSTANT_MESSAGE.format(
- name=attr.name.text))]]
- return []
-
-
-def _is_string(attr, module_source_file):
- """Checks if the given attr is a string."""
- if not attr.value.HasField("string_constant"):
- return [[error.error(module_source_file,
- attr.value.source_location,
- _BAD_TYPE_MESSAGE.format(name=attr.name.text,
- type="a string"))]]
- return []
-
-
-def _is_valid_byte_order(attr, module_source_file):
- """Checks if the given attr is a valid byte_order."""
- return _is_string_from_list(attr, module_source_file,
- {"BigEndian", "LittleEndian", "Null"})
-
-
-def _is_string_from_list(attr, module_source_file, valid_values):
- """Checks if the given attr has one of the valid_values."""
- if attr.value.string_constant.text not in valid_values:
- return [[error.error(module_source_file,
- attr.value.source_location,
- "Attribute '{name}' must be '{options}'.".format(
- name=attr.name.text,
- options="' or '".join(sorted(valid_values))))]]
- return []
-
-
-def _is_valid_text_output(attr, module_source_file):
- """Checks if the given attr is a valid text_output."""
- return _is_string_from_list(attr, module_source_file, {"Emit", "Skip"})
-
-
# Attributes must be the same type no matter where they occur.
_ATTRIBUTE_TYPES = {
- ("", attributes.ADDRESSABLE_UNIT_SIZE): _is_constant_integer,
- ("", attributes.BYTE_ORDER): _is_valid_byte_order,
- ("", attributes.ENUM_MAXIMUM_BITS): _is_constant_integer,
- ("", attributes.FIXED_SIZE): _is_constant_integer,
- ("", attributes.IS_INTEGER): _is_constant_boolean,
- ("", attributes.IS_SIGNED): _is_constant_boolean,
- ("", attributes.REQUIRES): _is_boolean,
- ("", attributes.STATIC_REQUIREMENTS): _is_boolean,
- ("", attributes.TEXT_OUTPUT): _is_valid_text_output,
- ("cpp", _NAMESPACE): _is_string,
+ attributes.ADDRESSABLE_UNIT_SIZE: attribute_util.INTEGER_CONSTANT,
+ attributes.BYTE_ORDER: _VALID_BYTE_ORDER,
+ attributes.ENUM_MAXIMUM_BITS: attribute_util.INTEGER_CONSTANT,
+ attributes.FIXED_SIZE: attribute_util.INTEGER_CONSTANT,
+ attributes.IS_INTEGER: attribute_util.BOOLEAN_CONSTANT,
+ attributes.IS_SIGNED: attribute_util.BOOLEAN_CONSTANT,
+ attributes.REQUIRES: attribute_util.BOOLEAN,
+ attributes.STATIC_REQUIREMENTS: attribute_util.BOOLEAN,
+ attributes.TEXT_OUTPUT: _VALID_TEXT_OUTPUT,
+ attributes.BACK_ENDS: _valid_back_ends,
}
_MODULE_ATTRIBUTES = {
- ("", attributes.BYTE_ORDER, True),
- # TODO(bolms): Allow back-end-specific attributes to be specified
- # externally.
- ("cpp", _NAMESPACE, False),
+ (attributes.BYTE_ORDER, True),
+ (attributes.BACK_ENDS, False),
}
_BITS_ATTRIBUTES = {
- ("", attributes.FIXED_SIZE, False),
- ("", attributes.REQUIRES, False),
+ (attributes.FIXED_SIZE, False),
+ (attributes.REQUIRES, False),
}
_STRUCT_ATTRIBUTES = {
- ("", attributes.FIXED_SIZE, False),
- ("", attributes.BYTE_ORDER, True),
- ("", attributes.REQUIRES, False),
+ (attributes.FIXED_SIZE, False),
+ (attributes.BYTE_ORDER, True),
+ (attributes.REQUIRES, False),
}
_ENUM_ATTRIBUTES = {
- ("", attributes.ENUM_MAXIMUM_BITS, False),
- ("", attributes.IS_SIGNED, False),
+ (attributes.ENUM_MAXIMUM_BITS, False),
+ (attributes.IS_SIGNED, False),
}
_EXTERNAL_ATTRIBUTES = {
- ("", attributes.ADDRESSABLE_UNIT_SIZE, False),
- ("", attributes.FIXED_SIZE, False),
- ("", attributes.IS_INTEGER, False),
- ("", attributes.STATIC_REQUIREMENTS, False),
+ (attributes.ADDRESSABLE_UNIT_SIZE, False),
+ (attributes.FIXED_SIZE, False),
+ (attributes.IS_INTEGER, False),
+ (attributes.STATIC_REQUIREMENTS, False),
}
_STRUCT_PHYSICAL_FIELD_ATTRIBUTES = {
- ("", attributes.BYTE_ORDER, False),
- ("", attributes.REQUIRES, False),
- ("", attributes.TEXT_OUTPUT, False),
+ (attributes.BYTE_ORDER, False),
+ (attributes.REQUIRES, False),
+ (attributes.TEXT_OUTPUT, False),
}
_STRUCT_VIRTUAL_FIELD_ATTRIBUTES = {
- ("", attributes.REQUIRES, False),
- ("", attributes.TEXT_OUTPUT, False),
+ (attributes.REQUIRES, False),
+ (attributes.TEXT_OUTPUT, False),
}
@@ -204,129 +149,6 @@
source_location=source_location)
-def _check_attributes_in_ir(ir):
- """Performs basic checks on all attributes in the given ir.
-
- This function calls _check_attributes on each attribute list in ir.
-
- Arguments:
- ir: An ir_pb2.EmbossIr to check.
-
- Returns:
- A list of lists of error.error, or an empty list if there were no errors.
- """
-
- def check_module(module, errors):
- errors.extend(_check_attributes(
- module.attribute, _MODULE_ATTRIBUTES, "module '{}'".format(
- module.source_file_name), module.source_file_name))
-
- def check_type_definition(type_definition, source_file_name, errors):
- if type_definition.HasField("structure"):
- if type_definition.addressable_unit == ir_pb2.TypeDefinition.BYTE:
- errors.extend(_check_attributes(
- type_definition.attribute, _STRUCT_ATTRIBUTES, "struct '{}'".format(
- type_definition.name.name.text), source_file_name))
- elif type_definition.addressable_unit == ir_pb2.TypeDefinition.BIT:
- errors.extend(_check_attributes(
- type_definition.attribute, _BITS_ATTRIBUTES, "bits '{}'".format(
- type_definition.name.name.text), source_file_name))
- else:
- assert False, "Unexpected addressable_unit '{}'".format(
- type_definition.addressable_unit)
- elif type_definition.HasField("enumeration"):
- errors.extend(_check_attributes(
- type_definition.attribute, _ENUM_ATTRIBUTES, "enum '{}'".format(
- type_definition.name.name.text), source_file_name))
- elif type_definition.HasField("external"):
- errors.extend(_check_attributes(
- type_definition.attribute, _EXTERNAL_ATTRIBUTES,
- "external '{}'".format(
- type_definition.name.name.text), source_file_name))
-
- def check_struct_field(field, source_file_name, errors):
- if ir_util.field_is_virtual(field):
- field_attributes = _STRUCT_VIRTUAL_FIELD_ATTRIBUTES
- field_adjective = "virtual "
- else:
- field_attributes = _STRUCT_PHYSICAL_FIELD_ATTRIBUTES
- field_adjective = ""
- errors.extend(_check_attributes(
- field.attribute, field_attributes,
- "{}struct field '{}'".format(field_adjective, field.name.name.text),
- source_file_name))
-
- errors = []
- # TODO(bolms): Add a check that only known $default'ed attributes are
- # used.
- traverse_ir.fast_traverse_ir_top_down(
- ir, [ir_pb2.Module], check_module,
- parameters={"errors": errors})
- traverse_ir.fast_traverse_ir_top_down(
- ir, [ir_pb2.TypeDefinition], check_type_definition,
- parameters={"errors": errors})
- traverse_ir.fast_traverse_ir_top_down(
- ir, [ir_pb2.Field], check_struct_field,
- parameters={"errors": errors})
- return errors
-
-
-def _check_attributes(attribute_list, attribute_specs, context_name,
- module_source_file):
- """Performs basic checks on the given list of attributes.
-
- Checks the given attribute_list for duplicates, unknown attributes, attributes
- with incorrect type, and attributes whose values are not constant.
-
- Arguments:
- attribute_list: An iterable of ir_pb2.Attribute.
- attribute_specs: A dict of attribute names to _Attribute structures
- specifying the allowed attributes.
- context_name: A name for the context of these attributes, such as "struct
- 'Foo'" or "module 'm.emb'". Used in error messages.
- module_source_file: The value of module.source_file_name from the module
- containing 'attribute_list'. Used in error messages.
-
- Returns:
- A list of lists of error.Errors. An empty list indicates no errors were
- found.
- """
- errors = []
- already_seen_attributes = {}
- for attr in attribute_list:
- if attr.back_end.text:
- attribute_name = "({}) {}".format(attr.back_end.text, attr.name.text)
- else:
- attribute_name = attr.name.text
- if (attr.name.text, attr.is_default) in already_seen_attributes:
- original_attr = already_seen_attributes[attr.name.text, attr.is_default]
- errors.append([
- error.error(module_source_file,
- attr.source_location,
- "Duplicate attribute '{}'.".format(attribute_name)),
- error.note(module_source_file,
- original_attr.source_location,
- "Original attribute")])
- continue
- already_seen_attributes[attr.name.text, attr.is_default] = attr
-
- if ((attr.back_end.text, attr.name.text, attr.is_default) not in
- attribute_specs):
- if attr.is_default:
- error_message = "Attribute '{}' may not be defaulted on {}.".format(
- attribute_name, context_name)
- else:
- error_message = "Unknown attribute '{}' on {}.".format(attribute_name,
- context_name)
- errors.append([error.error(module_source_file,
- attr.name.source_location,
- error_message)])
- else:
- attribute_check = _ATTRIBUTE_TYPES[attr.back_end.text, attr.name.text]
- errors.extend(attribute_check(attr, module_source_file))
- return errors
-
-
def _fixed_size_of_struct_or_bits(struct, unit_size):
"""Returns size of struct in bits or None, if struct is not fixed size."""
size = 0
@@ -431,6 +253,24 @@
field.source_location)])
+def _add_missing_back_ends_to_module(module):
+ """Sets the expected_back_ends attribute for a module, if not already set."""
+ back_ends_attr = ir_util.get_attribute(module.attribute, attributes.BACK_ENDS)
+ if back_ends_attr is None:
+ module.attribute.extend(
+ [_construct_string_attribute(attributes.BACK_ENDS, _DEFAULT_BACK_ENDS,
+ module.source_location)])
+
+
+def _gather_expected_back_ends(module):
+ """Captures the expected_back_ends attribute for `module`."""
+ back_ends_attr = ir_util.get_attribute(module.attribute, attributes.BACK_ENDS)
+ back_ends_str = back_ends_attr.string_constant.text
+ return {
+ "expected_back_ends": {x.strip() for x in back_ends_str.split(",")} | {""}
+ }
+
+
def _add_addressable_unit_to_external(external, type_definition):
"""Sets the addressable_unit field for an external TypeDefinition."""
# Strictly speaking, addressable_unit isn't an "attribute," but it's close
@@ -571,6 +411,8 @@
def _add_missing_attributes_on_ir(ir):
"""Adds missing attributes in a complete IR."""
traverse_ir.fast_traverse_ir_top_down(
+ ir, [ir_pb2.Module], _add_missing_back_ends_to_module)
+ traverse_ir.fast_traverse_ir_top_down(
ir, [ir_pb2.External], _add_addressable_unit_to_external)
traverse_ir.fast_traverse_ir_top_down(
ir, [ir_pb2.Enum], _add_missing_width_and_sign_attributes_on_enum)
@@ -600,10 +442,35 @@
_verify_requires_attribute_on_field(field, source_file_name, ir, errors)
+def _verify_back_end_attributes(attribute, expected_back_ends, source_file_name,
+ ir, errors):
+ back_end_text = attribute.back_end.text
+ if back_end_text not in expected_back_ends:
+ expected_back_ends_for_error = expected_back_ends - {""}
+ errors.append([error.error(
+ source_file_name, attribute.back_end.source_location,
+ "Back end specifier '{back_end}' does not match any expected back end "
+ "specifier for this file: '{expected_back_ends}'. Add or update the "
+ "'[expected_back_ends: \"{new_expected_back_ends}\"]' attribute at the "
+ "file level if this back end specifier is intentional.".format(
+ back_end=attribute.back_end.text,
+ expected_back_ends="', '".join(
+ sorted(expected_back_ends_for_error)),
+ new_expected_back_ends=", ".join(
+ sorted(expected_back_ends_for_error | {back_end_text})),
+ ))])
+
+
def _verify_attributes_on_ir(ir):
"""Verifies attributes in a complete IR."""
errors = []
traverse_ir.fast_traverse_ir_top_down(
+ ir, [ir_pb2.Attribute], _verify_back_end_attributes,
+ incidental_actions={
+ ir_pb2.Module: _gather_expected_back_ends,
+ },
+ parameters={"errors": errors})
+ traverse_ir.fast_traverse_ir_top_down(
ir, [ir_pb2.Structure], _verify_size_attributes_on_structure,
parameters={"errors": errors})
traverse_ir.fast_traverse_ir_top_down(
@@ -632,7 +499,16 @@
Returns:
A list of validation errors, or an empty list if no errors were encountered.
"""
- errors = _check_attributes_in_ir(ir)
+ errors = attribute_util.check_attributes_in_ir(
+ ir,
+ types=_ATTRIBUTE_TYPES,
+ module_attributes=_MODULE_ATTRIBUTES,
+ struct_attributes=_STRUCT_ATTRIBUTES,
+ bits_attributes=_BITS_ATTRIBUTES,
+ enum_attributes=_ENUM_ATTRIBUTES,
+ external_attributes=_EXTERNAL_ATTRIBUTES,
+ structure_virtual_field_attributes=_STRUCT_VIRTUAL_FIELD_ATTRIBUTES,
+ structure_physical_field_attributes=_STRUCT_PHYSICAL_FIELD_ATTRIBUTES)
if errors:
return errors
_add_missing_attributes_on_ir(ir)
diff --git a/compiler/front_end/attribute_checker_test.py b/compiler/front_end/attribute_checker_test.py
index efae0e6..def86c6 100644
--- a/compiler/front_end/attribute_checker_test.py
+++ b/compiler/front_end/attribute_checker_test.py
@@ -267,22 +267,6 @@
]],
attribute_checker.normalize_and_verify(ir))
- def test_accepts_string_attribute(self):
- ir = _make_ir_from_emb('[(cpp) namespace: "foo"]\n')
- self.assertEqual([], attribute_checker.normalize_and_verify(ir))
-
- def test_rejects_wrong_type_for_string_attribute(self):
- ir = _make_ir_from_emb("[(cpp) namespace: 9]\n")
- attr = ir.module[0].attribute[0]
- self.assertEqual([[
- error.error("m.emb", attr.value.source_location,
- "Attribute 'namespace' must have a string value.")
- ]], attribute_checker.normalize_and_verify(ir))
-
- def test_accepts_back_end_qualified_attribute(self):
- ir = _make_ir_from_emb('[(cpp) namespace: "abc"]\n')
- self.assertEqual([], attribute_checker.normalize_and_verify(ir))
-
def test_rejects_attribute_missing_required_back_end_specifier(self):
ir = _make_ir_from_emb('[namespace: "abc"]\n')
attr = ir.module[0].attribute[0]
@@ -291,22 +275,63 @@
"Unknown attribute 'namespace' on module 'm.emb'.")
]], attribute_checker.normalize_and_verify(ir))
- def test_rejects_attribute_with_wrong_back_end_specifier(self):
- ir = _make_ir_from_emb('[(c) namespace: "abc"]\n')
+ def test_accepts_attribute_with_default_known_back_end_specifier(self):
+ ir = _make_ir_from_emb('[(cpp) namespace: "abc"]\n')
+ self.assertEqual([], attribute_checker.normalize_and_verify(ir))
+
+ def test_rejects_attribute_with_specified_back_end_specifier(self):
+ ir = _make_ir_from_emb('[(c) namespace: "abc"]\n'
+ '[expected_back_ends: "c, cpp"]\n')
+ self.assertEqual([], attribute_checker.normalize_and_verify(ir))
+
+ def test_rejects_cpp_backend_attribute_when_not_in_expected_back_ends(self):
+ ir = _make_ir_from_emb('[(cpp) namespace: "abc"]\n'
+ '[expected_back_ends: "c"]\n')
attr = ir.module[0].attribute[0]
+ self.maxDiff = 200000
self.assertEqual([[
- error.error("m.emb", attr.name.source_location,
- "Unknown attribute '(c) namespace' on module 'm.emb'.")
+ error.error(
+ "m.emb", attr.back_end.source_location,
+ "Back end specifier 'cpp' does not match any expected back end "
+ "specifier for this file: 'c'. Add or update the "
+ "'[expected_back_ends: \"c, cpp\"]' attribute at the file level if "
+ "this back end specifier is intentional.")
]], attribute_checker.normalize_and_verify(ir))
- def test_rejects_emboss_internal_attribute_with_back_end_specifier(self):
- ir = _make_ir_from_emb('[(cpp) byte_order: "LittleEndian"]\n')
+ def test_rejects_expected_back_ends_with_bad_back_end(self):
+ ir = _make_ir_from_emb('[expected_back_ends: "c++"]\n')
attr = ir.module[0].attribute[0]
self.assertEqual([[
- error.error("m.emb", attr.name.source_location,
- "Unknown attribute '(cpp) byte_order' on module 'm.emb'.")
+ error.error(
+ "m.emb", attr.value.source_location,
+ "Attribute 'expected_back_ends' must be a comma-delimited list of "
+ "back end specifiers (like \"cpp, proto\")), not \"c++\".")
]], attribute_checker.normalize_and_verify(ir))
+ def test_rejects_expected_back_ends_with_no_comma(self):
+ ir = _make_ir_from_emb('[expected_back_ends: "cpp z"]\n')
+ attr = ir.module[0].attribute[0]
+ self.assertEqual([[
+ error.error(
+ "m.emb", attr.value.source_location,
+ "Attribute 'expected_back_ends' must be a comma-delimited list of "
+ "back end specifiers (like \"cpp, proto\")), not \"cpp z\".")
+ ]], attribute_checker.normalize_and_verify(ir))
+
+ def test_rejects_expected_back_ends_with_extra_commas(self):
+ ir = _make_ir_from_emb('[expected_back_ends: "cpp,,z"]\n')
+ attr = ir.module[0].attribute[0]
+ self.assertEqual([[
+ error.error(
+ "m.emb", attr.value.source_location,
+ "Attribute 'expected_back_ends' must be a comma-delimited list of "
+ "back end specifiers (like \"cpp, proto\")), not \"cpp,,z\".")
+ ]], attribute_checker.normalize_and_verify(ir))
+
+ def test_accepts_empty_expected_back_ends(self):
+ ir = _make_ir_from_emb('[expected_back_ends: ""]\n')
+ self.assertEqual([], attribute_checker.normalize_and_verify(ir))
+
def test_adds_byte_order_attributes_from_default(self):
ir = _make_ir_from_emb('[$default byte_order: "BigEndian"]\n'
"struct Foo:\n"
diff --git a/compiler/front_end/attributes.py b/compiler/front_end/attributes.py
index f931f5b..e4561da 100644
--- a/compiler/front_end/attributes.py
+++ b/compiler/front_end/attributes.py
@@ -25,3 +25,4 @@
TEXT_OUTPUT = "text_output"
ENUM_MAXIMUM_BITS = "maximum_bits"
IS_SIGNED = "is_signed"
+BACK_ENDS = "expected_back_ends"
diff --git a/compiler/front_end/emboss_front_end.py b/compiler/front_end/emboss_front_end.py
index ed435c2..6388128 100644
--- a/compiler/front_end/emboss_front_end.py
+++ b/compiler/front_end/emboss_front_end.py
@@ -86,11 +86,11 @@
return parser.parse_args(argv[1:])
-def _show_errors(errors, debug_info, flags):
+def _show_errors(errors, ir, flags):
"""Prints errors with source code snippets."""
source_codes = {}
- for source_file in debug_info.modules:
- source_codes[source_file] = debug_info.modules[source_file].source_code
+ for module in ir.module:
+ source_codes[module.source_file_name] = module.source_text
use_color = (flags.color_output == "always" or
(flags.color_output in ("auto", "if_tty") and
os.isatty(sys.stderr.fileno())))
@@ -132,7 +132,7 @@
ir, debug_info, errors = glue.parse_emboss_file(
flags.input_file[0], _find_in_dirs_and_read(flags.import_dirs))
if errors:
- _show_errors(errors, debug_info, flags)
+ _show_errors(errors, flags)
return 1
main_module_debug_info = debug_info.modules[flags.input_file[0]]
if flags.debug_show_tokenization:
diff --git a/compiler/front_end/glue.py b/compiler/front_end/glue.py
index 261027d..c19385d 100644
--- a/compiler/front_end/glue.py
+++ b/compiler/front_end/glue.py
@@ -160,6 +160,7 @@
debug_info.parse_tree = parse_result.parse_tree
used_productions = set()
ir = module_ir.build_ir(parse_result.parse_tree, used_productions)
+ ir.source_text = source_code
debug_info.used_productions = used_productions
debug_info.ir = ir_pb2.Module()
debug_info.ir.CopyFrom(ir)
diff --git a/compiler/front_end/module_ir_test.py b/compiler/front_end/module_ir_test.py
index 0bbd46c..1700888 100644
--- a/compiler/front_end/module_ir_test.py
+++ b/compiler/front_end/module_ir_test.py
@@ -27,11 +27,10 @@
from compiler.util import ir_pb2
_TESTDATA_PATH = "testdata.golden"
+_MINIMAL_SOURCE = pkgutil.get_data(
+ _TESTDATA_PATH, "span_se_log_file_status.emb").decode(encoding="UTF-8")
_MINIMAL_SAMPLE = parser.parse_module(
- tokenizer.tokenize(
- pkgutil.get_data(_TESTDATA_PATH, "span_se_log_file_status.emb").decode(
- encoding="UTF-8"),
- "")[0]).parse_tree
+ tokenizer.tokenize(_MINIMAL_SOURCE, "")[0]).parse_tree
_MINIMAL_SAMPLE_IR = ir_pb2.Module.from_json(
pkgutil.get_data(_TESTDATA_PATH, "span_se_log_file_status.ir.txt").decode(
encoding="UTF-8")
@@ -4031,7 +4030,9 @@
"""Tests the module_ir.build_ir() function."""
def test_build_ir(self):
- self.assertEqual(module_ir.build_ir(_MINIMAL_SAMPLE), _MINIMAL_SAMPLE_IR)
+ ir = module_ir.build_ir(_MINIMAL_SAMPLE)
+ ir.source_text = _MINIMAL_SOURCE
+ self.assertEqual(ir, _MINIMAL_SAMPLE_IR)
def test_production_coverage(self):
"""Checks that all grammar productions are used somewhere in tests."""
diff --git a/compiler/util/BUILD b/compiler/util/BUILD
index 92f4cde..c9ec128 100644
--- a/compiler/util/BUILD
+++ b/compiler/util/BUILD
@@ -47,8 +47,19 @@
python_version = "PY3",
deps = [
":expression_parser",
- ":ir_util",
":ir_pb2",
+ ":ir_util",
+ ],
+)
+
+py_library(
+ name = "attribute_util",
+ srcs = ["attribute_util.py"],
+ deps = [
+ ":error",
+ ":ir_pb2",
+ ":ir_util",
+ ":traverse_ir",
],
)
diff --git a/compiler/util/attribute_util.py b/compiler/util/attribute_util.py
new file mode 100644
index 0000000..80387bc
--- /dev/null
+++ b/compiler/util/attribute_util.py
@@ -0,0 +1,275 @@
+# Copyright 2019 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Module which verifies attributes in an Emboss IR.
+
+The main entry point is check_attributes_in_ir(), which checks attributes in an
+IR.
+"""
+
+from compiler.util import error
+from compiler.util import ir_pb2
+from compiler.util import ir_util
+from compiler.util import traverse_ir
+
+
+# Error messages used by multiple attribute type checkers.
+_BAD_TYPE_MESSAGE = "Attribute '{name}' must have {type} value."
+_MUST_BE_CONSTANT_MESSAGE = "Attribute '{name}' must have a constant value."
+
+
+def _attribute_name_for_errors(attr):
+ if attr.back_end.text:
+ return f"({attr.back_end.text}) {attr.name.text}"
+ else:
+ return attr.name.text
+
+
+# Attribute type checkers
+def _is_constant_boolean(attr, module_source_file):
+ """Checks if the given attr is a constant boolean."""
+ if not attr.value.expression.type.boolean.HasField("value"):
+ return [[error.error(module_source_file,
+ attr.value.source_location,
+ _BAD_TYPE_MESSAGE.format(
+ name=_attribute_name_for_errors(attr),
+ type="a constant boolean"))]]
+ return []
+
+
+def _is_boolean(attr, module_source_file):
+ """Checks if the given attr is a boolean."""
+ if attr.value.expression.type.WhichOneof("type") != "boolean":
+ return [[error.error(module_source_file,
+ attr.value.source_location,
+ _BAD_TYPE_MESSAGE.format(
+ name=_attribute_name_for_errors(attr),
+ type="a boolean"))]]
+ return []
+
+
+def _is_constant_integer(attr, module_source_file):
+ """Checks if the given attr is an integer constant expression."""
+ if (not attr.value.HasField("expression") or
+ attr.value.expression.type.WhichOneof("type") != "integer"):
+ return [[error.error(module_source_file,
+ attr.value.source_location,
+ _BAD_TYPE_MESSAGE.format(
+ name=_attribute_name_for_errors(attr),
+ type="an integer"))]]
+ if not ir_util.is_constant(attr.value.expression):
+ return [[error.error(module_source_file,
+ attr.value.source_location,
+ _MUST_BE_CONSTANT_MESSAGE.format(
+ name=_attribute_name_for_errors(attr)))]]
+ return []
+
+
+def _is_string(attr, module_source_file):
+ """Checks if the given attr is a string."""
+ if not attr.value.HasField("string_constant"):
+ return [[error.error(module_source_file,
+ attr.value.source_location,
+ _BAD_TYPE_MESSAGE.format(
+ name=_attribute_name_for_errors(attr),
+ type="a string"))]]
+ return []
+
+
+# Provide more readable names for these functions when used in attribute type
+# specifiers.
+BOOLEAN_CONSTANT = _is_constant_boolean
+BOOLEAN = _is_boolean
+INTEGER_CONSTANT = _is_constant_integer
+STRING = _is_string
+
+
+def string_from_list(valid_values):
+ """Checks if the given attr has one of the valid_values."""
+ def _string_from_list(attr, module_source_file):
+ if attr.value.string_constant.text not in valid_values:
+ return [[error.error(module_source_file,
+ attr.value.source_location,
+ "Attribute '{name}' must be '{options}'.".format(
+ name=_attribute_name_for_errors(attr),
+ options="' or '".join(sorted(valid_values))))]]
+ return []
+ return _string_from_list
+
+
+def check_attributes_in_ir(ir,
+ *,
+ back_end=None,
+ types=None,
+ module_attributes=None,
+ struct_attributes=None,
+ bits_attributes=None,
+ enum_attributes=None,
+ external_attributes=None,
+ structure_virtual_field_attributes=None,
+ structure_physical_field_attributes=None):
+ """Performs basic checks on all attributes in the given ir.
+
+ This function calls _check_attributes on each attribute list in ir.
+
+ Arguments:
+ ir: An ir_pb2.EmbossIr to check.
+ back_end: A string specifying the attribute qualifier to check (such as
+ `cpp` for `[(cpp) namespace = "foo"]`), or None to check unqualified
+ attributes.
+
+ Attributes with a different qualifier will not be checked.
+ types: A map from attribute names to validators, such as:
+ {
+ "maximum_bits": attribute_util.INTEGER_CONSTANT,
+ "requires": attribute_util.BOOLEAN,
+ }
+ module_attributes: A set of (attribute_name, is_default) tuples specifying
+ the attributes that are allowed at module scope.
+ struct_attributes: A set of (attribute_name, is_default) tuples specifying
+ the attributes that are allowed at `struct` scope.
+ bits_attributes: A set of (attribute_name, is_default) tuples specifying
+ the attributes that are allowed at `bits` scope.
+ enum_attributes: A set of (attribute_name, is_default) tuples specifying
+ the attributes that are allowed at `enum` scope.
+ external_attributes: A set of (attribute_name, is_default) tuples
+ specifying the attributes that are allowed at `external` scope.
+ structure_virtual_field_attributes: A set of (attribute_name, is_default)
+ tuples specifying the attributes that are allowed at the scope of
+ virtual fields (`let` fields) in structures (both `struct` and `bits`).
+ structure_physical_field_attributes: A set of (attribute_name, is_default)
+ tuples specifying the attributes that are allowed at the scope of
+ physical fields in structures (both `struct` and `bits`).
+
+ Returns:
+ A list of lists of error.error, or an empty list if there were no errors.
+ """
+
+ def check_module(module, errors):
+ errors.extend(_check_attributes(
+ module.attribute, types, back_end, module_attributes,
+ "module '{}'".format(
+ module.source_file_name), module.source_file_name))
+
+ def check_type_definition(type_definition, source_file_name, errors):
+ if type_definition.HasField("structure"):
+ if type_definition.addressable_unit == ir_pb2.TypeDefinition.BYTE:
+ errors.extend(_check_attributes(
+ type_definition.attribute, types, back_end, struct_attributes,
+ "struct '{}'".format(
+ type_definition.name.name.text), source_file_name))
+ elif type_definition.addressable_unit == ir_pb2.TypeDefinition.BIT:
+ errors.extend(_check_attributes(
+ type_definition.attribute, types, back_end, bits_attributes,
+ "bits '{}'".format(
+ type_definition.name.name.text), source_file_name))
+ else:
+ assert False, "Unexpected addressable_unit '{}'".format(
+ type_definition.addressable_unit)
+ elif type_definition.HasField("enumeration"):
+ errors.extend(_check_attributes(
+ type_definition.attribute, types, back_end, enum_attributes,
+ "enum '{}'".format(
+ type_definition.name.name.text), source_file_name))
+ elif type_definition.HasField("external"):
+ errors.extend(_check_attributes(
+ type_definition.attribute, types, back_end, external_attributes,
+ "external '{}'".format(
+ type_definition.name.name.text), source_file_name))
+
+ def check_struct_field(field, source_file_name, errors):
+ if ir_util.field_is_virtual(field):
+ field_attributes = structure_virtual_field_attributes
+ field_adjective = "virtual "
+ else:
+ field_attributes = structure_physical_field_attributes
+ field_adjective = ""
+ errors.extend(_check_attributes(
+ field.attribute, types, back_end, field_attributes,
+ "{}struct field '{}'".format(field_adjective, field.name.name.text),
+ source_file_name))
+
+ errors = []
+ # TODO(bolms): Add a check that only known $default'ed attributes are
+ # used.
+ traverse_ir.fast_traverse_ir_top_down(
+ ir, [ir_pb2.Module], check_module,
+ parameters={"errors": errors})
+ traverse_ir.fast_traverse_ir_top_down(
+ ir, [ir_pb2.TypeDefinition], check_type_definition,
+ parameters={"errors": errors})
+ traverse_ir.fast_traverse_ir_top_down(
+ ir, [ir_pb2.Field], check_struct_field,
+ parameters={"errors": errors})
+ return errors
+
+
+def _check_attributes(attribute_list, types, back_end, attribute_specs,
+ context_name, module_source_file):
+ """Performs basic checks on the given list of attributes.
+
+ Checks the given attribute_list for duplicates, unknown attributes, attributes
+ with incorrect type, and attributes whose values are not constant.
+
+ Arguments:
+ attribute_list: An iterable of ir_pb2.Attribute.
+ back_end: The qualifier for attributes to check, or None.
+ attribute_specs: A dict of attribute names to _Attribute structures
+ specifying the allowed attributes.
+ context_name: A name for the context of these attributes, such as "struct
+ 'Foo'" or "module 'm.emb'". Used in error messages.
+ module_source_file: The value of module.source_file_name from the module
+ containing 'attribute_list'. Used in error messages.
+
+ Returns:
+ A list of lists of error.Errors. An empty list indicates no errors were
+ found.
+ """
+ if attribute_specs is None:
+ attribute_specs = []
+ errors = []
+ already_seen_attributes = {}
+ for attr in attribute_list:
+ if attr.back_end.text:
+ if attr.back_end.text != back_end:
+ continue
+ else:
+ if back_end is not None:
+ continue
+ attribute_name = _attribute_name_for_errors(attr)
+ if (attr.name.text, attr.is_default) in already_seen_attributes:
+ original_attr = already_seen_attributes[attr.name.text, attr.is_default]
+ errors.append([
+ error.error(module_source_file,
+ attr.source_location,
+ "Duplicate attribute '{}'.".format(attribute_name)),
+ error.note(module_source_file,
+ original_attr.source_location,
+ "Original attribute")])
+ continue
+ already_seen_attributes[attr.name.text, attr.is_default] = attr
+
+ if (attr.name.text, attr.is_default) not in attribute_specs:
+ if attr.is_default:
+ error_message = "Attribute '{}' may not be defaulted on {}.".format(
+ attribute_name, context_name)
+ else:
+ error_message = "Unknown attribute '{}' on {}.".format(attribute_name,
+ context_name)
+ errors.append([error.error(module_source_file,
+ attr.name.source_location,
+ error_message)])
+ else:
+ errors.extend(types[attr.name.text](attr, module_source_file))
+ return errors
diff --git a/compiler/util/ir_pb2.py b/compiler/util/ir_pb2.py
index e175349..ef081bc 100644
--- a/compiler/util/ir_pb2.py
+++ b/compiler/util/ir_pb2.py
@@ -991,8 +991,9 @@
type = Repeated(TypeDefinition) # Module-level type definitions.
documentation = Repeated(Documentation) # Module-level docs.
foreign_import = Repeated(Import) # Other modules imported.
- source_location = Optional(Location) # Source code covered by this IR.
- source_file_name = Optional(_Text) # Name of the source file.
+ source_text = Optional(_Text) # The original source code.
+ source_location = Optional(Location) # Source code covered by this IR.
+ source_file_name = Optional(_Text) # Name of the source file.
@message
diff --git a/testdata/golden/span_se_log_file_status.ir.txt b/testdata/golden/span_se_log_file_status.ir.txt
index 942b49a..b1bfef8 100644
--- a/testdata/golden/span_se_log_file_status.ir.txt
+++ b/testdata/golden/span_se_log_file_status.ir.txt
@@ -1083,5 +1083,6 @@
"column": 1
},
"is_synthetic": false
- }
+ },
+ "source_text": "# Copyright 2019 Google LLC\n#\n# Licensed under the Apache License, Version 2.0 (the \"License\");\n# you may not use this file except in compliance with the License.\n# You may obtain a copy of the License at\n#\n# https://www.apache.org/licenses/LICENSE-2.0\n#\n# Unless required by applicable law or agreed to in writing, software\n# distributed under the License is distributed on an \"AS IS\" BASIS,\n# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n# See the License for the specific language governing permissions and\n# limitations under the License.\n\n-- This is a simple, real-world example structure.\n\n[$default byte_order: \"LittleEndian\"]\n[(cpp) namespace: \"emboss::test\"]\n\n\nstruct LogFileStatus:\n 0 [+4] UInt file_state\n 4 [+12] UInt:8[12] file_name\n 16 [+4] UInt file_size_kb\n 20 [+4] UInt media\n"
}
