-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: prepare public API for alpha release #391
Conversation
2e3296a
to
5a69f34
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some general issues with the semantics of the renamings in this PR.
created a slang_solidity::syntax
module, with sub-modules nodes
, parsing
, and visitors
nodes
and visitors
are nouns, but parsing
is a verb.
renamed codegen_parser
to codegen_syntax
accourdingly
Note sure about this for the same reason. It does generate the parser. I know we discussed this, but now that I see it, I'm not convinced. I think it makes it less semantically clear.
renamed Visitable::visit()
to Receiver::receive()
for correctness
IMO Visitable
is much more descriptive than Receiver
because it ties it to the Visitor
. I understand the linguistic relationship of receiving a visitor, but Receiver
seems too generic (Sender
/Receiver
), and the -able
suffix is better linguistically for me. The only other alternative that seems right is ReceiveVisitor::receive_visitor
, but I strongly prefer -able
for the name of this role in these kind of patterns.
renamed ParserOutput
to ParseResult
to match
I see ...Result
and I think Result<T>
, with the corresponding type semantics. Which ParserOutput
is not.
f2f: let's use
f2f: let's use
f2f: let's use
f2f: let's use |
Should also add rust-lang/cargo#1982 to the dummy module comment. |
- created a `slang_solidity::syntax` module, with sub-modules `nodes`, `parser`, and `visitors`. - renamed `codegen_parser` to `codegen_syntax` accourdingly. - renamed `Visitable::visit()` to `Visitable::accept_visitor()`. - made `Visitable::accept_visitor_with_path()` private, since it accepts a `&mut path`, and we expect/pop nodes in a certain order that external callers can violate. - renamed `ParserOutput` to `ParseOutput` to match `parse()`. - changed a few internal helpers/constructors to `pub(crate)` instead. Tested using `cargo rustdoc`.
5a69f34
to
a8f7cc8
Compare
Comments addressed, and PR description updated with changes. |
slang_solidity::syntax
module, with sub-modulesnodes
,parser
, andvisitors
.codegen_parser
tocodegen_syntax
accourdingly.Visitable::visit()
toVisitable::accept_visitor()
.Visitable::accept_visitor_with_path()
private, since it accepts a&mut path
, and we expect/pop nodes in a certain order that external callers can violate.ParserOutput
toParseOutput
to matchparse()
.pub(crate)
instead.Tested using
cargo rustdoc
.