-
Notifications
You must be signed in to change notification settings - Fork 14k
Rename eval_always query modifier to no_incremental
#146936
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
base: main
Are you sure you want to change the base?
Conversation
|
The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes. |
This comment has been minimized.
This comment has been minimized.
\`eval_always\` gives the impression that the query is evaluated every time it's called. (thus, evaluated always). Turns out this isn't the truth, it just affects how the dependency graph behaves with that node (i.e. not even trying to mark it green) This commit renames `eval_always` to `no_incremental`, I thought about `always_red_on_depgraph` or similar but I don't think that gatekeeping its use under minor details of how the dep graph works is that preferable.
fd130cd to
1732f04
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
I'm not convinced that this terminology is better. |
But this is not clear, in fact it's so unclear I was misguided just until yesterday :< maybe another name would be better, but If |
|
Query results are by default not stored on the disk. You have to use |
|
Okay, so I'd like to move this PR somewhere towards merge (in some form) or close. Sounds like @cjgillot has specific concerns about this PR, not sure if @bjorn3 had specific concerns or anything. @blyxyas what are your feelings here? Do you want to try to push for some kind of rename? Or would you rather just close? |
|
I prefer renaming it to another terminology than closing this, I really think that
|
eval_alwaysgives the impression that the query is evaluated every time it's called. (thus, evaluated always). Turns out this isn't the truth, it just affects how the dependency graph behaves with that node (i.e. not even trying to mark it green)This commit renames
eval_alwaystono_incremental, I thought aboutalways_red_on_depgraphor similar but I don't think that expecting every query author to know what "red" means in this context is preferable or useful.