-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Split up const_eval.rs
#67327
Split up const_eval.rs
#67327
Conversation
Btw... perhaps its time to consider moving const eval & interpretation out of librustc_mir into its own crate. (Not in this PR though, which is already doing god's work.) |
I don't entirely understand the separation between |
No, but I didn't feel like creating a deeper module structure right now, so I just split things by what they do, not necessarily increasing the modularity. |
I don't understand what the criterion is that puts stuff into Could you elaborate? |
|
There are other queries though, right? The field projection and downcast stuff? To my untrained eye, the separation between the two files looks pretty arbitrary, and I am not sure if it helps. It's useful to separate things that can be neatly separated as it makes files shorter and thus stuff easier to find, but it is IMO bad to separate things that are hard to tell apart as it'll just mean I will have to check both of these files every single time as I cannot predict which one a given function fits into. |
As an alternative split, what about putting everything needed by the two "main" queries into |
That seems reasonable |
Not sure if |
The other functions left in |
☔ The latest upstream changes (presumably #67532) made this pull request unmergeable. Please resolve the merge conflicts. |
👍 for the split. Any proposals for the |
|
Hm, |
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 |
r=me with tidy happy and the nit fixed. |
@bors r=RalfJung |
📌 Commit 3719f4b3b688b81808dbcf65431ba185ff32616f has been approved by |
🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened |
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 |
☔ The latest upstream changes (presumably #66919) made this pull request unmergeable. Please resolve the merge conflicts. |
@bors r=RalfJung |
📌 Commit 07df147 has been approved by |
☀️ Test successful - checks-azure |
Fixes #67316
r? @RalfJung