API: Wrap *Kind enums in *Kind enums
#244
Conversation
xFrednet
added
C-documentation
marker_api/src/ast/stmt.rs
Outdated
| #[cfg(feature = "driver-api")] | ||
| impl<'ast> ItemStmt<'ast> { | ||
| pub fn new(data: CommonStmtData<'ast>, item: ItemKind<'ast>) -> Self { | ||
| Self { data, item } | ||
| } | ||
| } |
There was a problem hiding this comment.
Could it make sense to just mark the fields of the structs conditionally pub instead?
There was a problem hiding this comment.
The visibility crate sadly doesn't work on field structs. Otherwise, that could be an option. I considered adding something like impl_new to cfg derive these things. I was a bit hesitant with Marker's FFI types, but I just saw that it uses impl Into<Ty> that could actually work 🤔
Edit: https://docs.rs/derive-new/0.5.9/derive_new/ is probably the safer choice, since impl_new is very new. Let's hope that one also has the Into<> thing 😅
There was a problem hiding this comment.
I wouldn't add a proc macro dependency to marker_api. I love the fact that it can be a zero dependency crate, meaning the lint crates can also be effectively zero-dep (except for marker_api), and compile blazingly fast
There was a problem hiding this comment.
The macro would only be needed when it's compiled with the driver-api feature, so it would remain zero-dep :D
There was a problem hiding this comment.
I'm undecided on this one. impl_new looks great but only has a total of 102 downloads. derive-new is from @nrc but doesn't seem to support Into<X> parameters, which is important for marker. It also doesn't seem like the crate isn't actively maintained, when I look at the issue tracker 🤔
Created: nrc/derive-new#62
There was a problem hiding this comment.
I'd recommend you to use either typed-builder, for more elaborate cases: buildstructor.
typed-builder has support for impl Into via #[builder(setter(into))] (opt-in), and buildstructor just slaps impl Into automatically (opt-out).
There was a problem hiding this comment.
I'll try using typed-builder instead of the newly added constructors, wish me luck!
There was a problem hiding this comment.
It seems to work quite well. I won't update everything rn, but I'll keep the dependency and use it for new types :)
Here are the changes: 367cfb5
Looks like Clippy doesn't like it though :/ I'll allow the lint and create an issue
There was a problem hiding this comment.
There was also another lint that caused problems clippy::used_underscore_binding (idanarye/rust-typed-builder#113)
Let's hope everything works now :)
Co-authored-by: Veetaha <gersoh3@gmail.com>
xFrednet
force-pushed
the
244-no-kind-in-kind
branch
from
10c1c0c
to
367cfb5
Compare
xFrednet
force-pushed
the
244-no-kind-in-kind
branch
from
367cfb5
to
84b4897
Compare
xFrednet
force-pushed
the
244-no-kind-in-kind
branch
from
84b4897
to
3ed653e
Compare
|
Thank you for the review @Veetaha, I highly appreciate it ❤️ |
The current API has a few spots where a
*Kindenum directly wraps a*Kindenum, like:PatKind::Lit. This is cool, for matching but causes a bunch of problems when it comes to common data shared among the outer*Kindenum. To be future-proof and have this consistent in the entire API, I've now wrapped the inner*Kindvalues in new structs.These structs are often just wrappers, but will help us in the future. This will help me a lot with #242
Most of the changes are just uilint tests.