search

PLC-lang / rusty

Structured Text Parser and LLVM Frontend
GNU Lesser General Public License v3.0
181 stars 47 forks source link

refactor: Merge `plc_source` and `plc_ast` crates #1217

Open volsa opened 2 weeks ago

volsa commented 2 weeks ago

The SourceLocation struct is part of the AstNode, thus being tightly coupled it makes sense to merge them into one crate namely into plc_ast.

Edit: The naming and structure of some files isn't perfect yet, but I thought I'll address this in another PR to not bloat the diff too much.

ghaith commented 2 weeks ago

The idea of splitting the 2 crates was that the reading source files does not directly relate to parsing the AST, as we get an AST by first reading the Source file, and then using the parser from one of the multiple crates (xml_parser/parser) to build an AST. I don't mind the 2 combined to be honest since the change is not that big, it's only pulling AST into project now, and you are right that the SourceLocation is tightly coupled with the AST, but other structs were not (SourceContainer/SourceCode). Is there a specific benefit you are looking for? Could it be done by only moving the SourceLocation?

I'm not opposed to the change, just thinking out loud.

volsa commented 2 weeks ago

Is there a specific benefit you are looking for?

None, other than structurally wherever the plc_ast crate was imported so was the plc_source.

Could it be done by only moving the SourceLocation?

I think this should work? Retrospectively I like this more than merging these two crates together considering the argument with the SourceContainer & SourceCode traits. I'll experiment with it and update the PR if it works. Back to draft mode it is 😄