google / emboss

Emboss is a tool for generating code that reads and writes binary data structures.
Apache License 2.0
68 stars 21 forks source link

Factor back-end attribute checking into the back end. #80

Closed reventlov closed 1 year ago

reventlov commented 1 year ago

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 allow attributes from unknown back ends to pass with no checking -- if you put an attribute [(my_kewl_back_end) i_love_ducks: "ducks are awesome!"] in an .emb file, that will now pass through the compiler without complaint.

AaronWebster commented 1 year ago

Would it be useful to the user to include an warning if the backend is unknown? It's not apparent what errors would be presented to the user in this case, especially with future non-c++ codegens.

AaronWebster commented 1 year ago

LGTM

reventlov commented 1 year ago

Would it be useful to the user to include an warning if the backend is unknown? It's not apparent what errors would be presented to the user in this case, especially with future non-c++ codegens.

I don't have a good solution to this one (and it is the main reason I hadn't moved back end attribute checking into the back end earlier). The two least-bad solutions I've come up with are:

In both cases, the front end would only be able to check that the cpp in [(cpp) name: "value"] wasn't misspelled -- it wouldn't be able to check that name and "value" were valid.