-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Define queries using a proc macro #56462
Conversation
r? @zackmdavis (rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
|
||
rustc_query_append! { [define_queries!][ <'tcx> |
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.
This is mind-bending, but cool!
This comment has been minimized.
This comment has been minimized.
92fcbc5
to
20b9c1b
Compare
This comment has been minimized.
This comment has been minimized.
@Zoxc This is cool :) When it merges, could you please update https://rust-lang.github.io/rustc-guide/query.html ? That would be much appreciated! |
This comment has been minimized.
This comment has been minimized.
Perhaps I didn't work with queries enough, but I don't understand what's the point of the change and why is this an improvement. |
@petrochenkov This is pretty much just replacing a The PR is more confusing atm though, because it kept both macros and daisy-chained them, to allow gradually moving from the old one to the new one. |
I forgot to include syntax for impl<'tcx> QueryDescription<'tcx> for queries::type_op_prove_predicate<'tcx> {
fn describe(_tcx: TyCtxt<'_, '_, '_>, goal: CanonicalTypeOpProvePredicateGoal<'tcx>)
-> Cow<'static, str> {
format!("evaluating `type_op_prove_predicate` `{:?}`", goal).into()
}
} Anyone have some good ideas for that? |
I'd say something like this: description("evaluating `type_op_prove_predicate` `{:?}`", goal) |
@eddyb That looks very unbound. How about: query type_of(DefId) -> Ty<'tcx> {
"evaluating `finding type of `{:?}`", $key
} |
Why not use a doc comment? |
@mark-i-m Literally this? /// "finding type of `{:?}`", $key`
Or something neater? Maybe like this: /// finding type of `{key:?}`
|
This comment has been minimized.
This comment has been minimized.
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
I don't think using comments is a good idea. You'd want to use those to document the queries. Also it's very surprising that comments are executable. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
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.
just a nit and a potential refactoring
r=me with both addressed
@bors r+ |
📌 Commit 198dfce has been approved by |
⌛ Testing commit 198dfce with merge 3a05002e245eb2c9fe06c1ab62ef48e0c7f0f69c... |
💥 Test timed out |
@bors retry |
Define queries using a proc macro cc @rust-lang/compiler
☀️ Test successful - checks-travis, status-appveyor |
There was a big macro rewrite in these pull requests: rust-lang/rust#56462 rust-lang/rust#59517 Update the query chapter to describe the new macro usage.
There was a big macro rewrite in these pull requests: rust-lang/rust#56462 rust-lang/rust#59517 Update the query chapter to describe the new macro usage.
There was a big macro rewrite in these pull requests: rust-lang/rust#56462 rust-lang/rust#59517 Update the query chapter to describe the new macro usage.
cc @rust-lang/compiler