-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Resolve the prelude import in build_reduced_graph
#145322
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: master
Are you sure you want to change the base?
Resolve the prelude import in build_reduced_graph
#145322
Conversation
Could you also add this case to tests? //@ check-pass
#![feature(custom_inner_attributes)]
#![rustfmt::skip]
fn main() {
let _ = todo!();
} |
I am currently unable to implement any of your suggestions; it is the prelude import that can not be resolved during |
@Voultapher |
Suggestions worked, however, we can't emit an error when the prelude is already set, I just used a panic for ease of use, and it was triggered when compiling stdlib:
I also can't avoid to only resolve the prelude path when the prelude is not set. Errors like these get emitted:
@rustbot ready |
Okay, I see, in |
@@ -421,6 +421,9 @@ | |||
// | |||
#![default_lib_allocator] | |||
|
|||
// The Rust prelude | |||
pub mod prelude; | |||
|
|||
// Explicitly import the prelude. The compiler uses this same unstable attribute | |||
// to import the prelude implicitly when building crates that depend on std. |
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.
Could you add a comment saying that this import should currently go after mod prelude
to work?
And same comment in libcore.
@@ -421,6 +421,9 @@ | |||
// | |||
#![default_lib_allocator] | |||
|
|||
// The Rust prelude | |||
pub mod prelude; | |||
|
|||
// Explicitly import the prelude. The compiler uses this same unstable attribute | |||
// to import the prelude implicitly when building crates that depend on std. | |||
#[prelude_import] |
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.
Is #[allow(unused)]
still needed on prelude imports?
If not, could you remove them in libcore and libstd?
@@ -0,0 +1,9 @@ | |||
//@ check-pass |
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.
Could you add a comment saying that the test checks that macro names resolved from libstd prelude still work if there's a crate level custom inner attribute?
And rename the test to something less generic.
if !ast::attr::contains_name(&item.attrs, sym::prelude_import) { | ||
let kind = ImportKind::Glob { max_vis: Cell::new(None), id }; | ||
self.add_import(prefix, kind, use_tree.span, item, root_span, item.id, vis); | ||
return; |
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.
When I see return
I have to look at the whole function to check if anything is skipped in the bottom due to that return
.
Just use if-else or if guard (with a positive condition first).
}; | ||
|
||
self.add_import(prefix, kind, use_tree.span, item, root_span, item.id, vis); | ||
if !ast::attr::contains_name(&item.attrs, sym::prelude_import) { |
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.
if !ast::attr::contains_name(&item.attrs, sym::prelude_import) { | |
if !attr::contains_name(&item.attrs, sym::prelude_import) { |
self.r.prelude = Some(module); | ||
} else { | ||
// Same as in `imports::resolve_glob_import` | ||
self.r.dcx().emit_err(CannotGlobImportAllCrates { span: use_tree.span }); |
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.
Well, using an entirely unrelated error is too much.
You can use a string, dcx.span_err(span, "cannot resolve a prelude import").emit()
, without introducing a new "translatable" error type.
The job Click to see the possible cause of the failure (guessed by this bot)
|
This pr tries to resolve the prelude import at the
build_reduced_graph
stage.Part of batched import resolution in #145108 (cherry picked commit) and maybe needed for #139493.
r? petrochenkov