zeek / spicy

C++ parser generator for dissecting protocols & files.
https://docs.zeek.org/projects/spicy
Other
248 stars 37 forks source link

Disallow attributes on "type aliases" in parser #1898

Open bbannier opened 17 hours ago

bbannier commented 17 hours ago

With #1890 we added a validator to reject any attributes on "type alias". To detect such type aliases we used a pretty nasty hack,

https://github.com/zeek/spicy/blob/500f2852f22e6dc01cd84ed717c0441c8483f47e/spicy/toolchain/src/compiler/validator.cc#L360-L363

We should clean this up by instead disallowing attributes on any declaration which is not a enum or unit (not sure there are more entities on the RHS where even with #1890 we support attributes). A way to do that would be to push the parsing of attributes from type_decl into e.g., unit_type; in fact the parser already has special handling for units,

https://github.com/zeek/spicy/blob/500f2852f22e6dc01cd84ed717c0441c8483f47e/spicy/toolchain/src/compiler/parser/parser.yy#L381-L385

With that the added validator would become redundant.

evantypanski commented 17 hours ago

Just as a minor point: This would result in an error in parsing that's just "unexpected token" which seems unintuitive when it really looks like these attributes apply to types. There would have to be a little extra handling to give a nice error message, which is exactly what the validator does. I kind of prefer that, but it's not impossible to get a nice error there in parsing. From what I can tell, units have the attributes apply to the type (hence that workaround), but enums have it apply to the decl, so there is another edge case I think?