-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
expand: Refactor module loading #82415
Conversation
r? @davidtwco (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
03a360d
to
0ea2e8b
Compare
☔ The latest upstream changes (presumably #82448) made this pull request unmergeable. Please resolve the merge conflicts. |
#![feature(crate_visibility_modifier)] | ||
#![feature(decl_macro)] | ||
#![feature(destructuring_assignment)] |
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 feature is currently not used anywhere inside the compiler. Given that you are only using it at one single place in rustc_expand and that avoiding it would the life of for example mrustc easier at the expense of writing just a tiny bit less nice code, I would err towards not using it.
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.
To be clear if using it had a bigger benefit for rustc itself, I would be fine with using it. It is just that I don't think the benefit for rustc outweighs the cost for mrustc in this particular case.
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 more or less rust-lang/compiler-team#358 again.
I want to use compiler for dogfooding recently implemented unstable features, without looking at what third-party projects may think about it.
Destructuring assignment is a convenient feature, if it's removed from this PR then it may be useful in a next PR.
Similarly, #82641 starts using #![feature(extended_key_value_attributes)]
, should it abandon it too?
At which point are we allowed to use these features? I don't want mrustc to decide that.
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.
Note that #![feature(extended_key_value_attributes)]
will probably be stabilized soon anyway.
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.
Sorry for the delay in getting to this, looks good to me, r=me after rebasing.
This ownership kind is only constructed in the case of path attributes like `#[path = ".."]` without a file name segment, which always represent some kind of directories and will produce and error on attempt to parse them as a module file.
…data Also don't push the paths on the stack directly in `fn parse_external_mod`, return them instead.
@bors r=davidtwco |
📌 Commit 1fe2eb8 has been approved by |
expand: Refactor module loading This is an accompanying PR to rust-lang#82399, but they can be landed independently. See individual commits for more details. Anyone should be able to review this equally well because all people actually familiar with this code left the project.
expand: Refactor module loading This is an accompanying PR to rust-lang#82399, but they can be landed independently. See individual commits for more details. Anyone should be able to review this equally well because all people actually familiar with this code left the project.
expand: Refactor module loading This is an accompanying PR to rust-lang#82399, but they can be landed independently. See individual commits for more details. Anyone should be able to review this equally well because all people actually familiar with this code left the project.
expand: Refactor module loading This is an accompanying PR to rust-lang#82399, but they can be landed independently. See individual commits for more details. Anyone should be able to review this equally well because all people actually familiar with this code left the project.
expand: Refactor module loading This is an accompanying PR to rust-lang#82399, but they can be landed independently. See individual commits for more details. Anyone should be able to review this equally well because all people actually familiar with this code left the project.
Rollup of 10 pull requests Successful merges: - rust-lang#82047 (bypass auto_da_alloc for metadata files) - rust-lang#82415 (expand: Refactor module loading) - rust-lang#82557 (Add natvis for Result, NonNull, CString, CStr, and Cow) - rust-lang#82613 (Remove Item::kind, use tagged enum. Rename variants to match) - rust-lang#82642 (Fix jemalloc usage on OSX) - rust-lang#82682 (Implement built-in attribute macro `#[cfg_eval]` + some refactoring) - rust-lang#82684 (Disable destination propagation on all mir-opt-levels) - rust-lang#82755 (Refactor confirm_builtin_call, remove partial if) - rust-lang#82857 (Edit ructc_ast_lowering docs) - rust-lang#82862 (Generalize Write impl for Vec<u8> to Vec<u8, A>) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This is an accompanying PR to #82399, but they can be landed independently.
See individual commits for more details.
Anyone should be able to review this equally well because all people actually familiar with this code left the project.