resolve: More groundwork for item_like_imports (RFC 1560)
#35776
Conversation
|
cc #35120 (tracking issue for RFC 1560) |
|
RFC 1560's diagnostics and corner cases (esp. w.r.t. ambiguous bindings) ended up being a bit trickier to implement than I expected (my prototype, #32213, cut some corners semantically). That being said, I believe this will be the final groundwork PR -- once this lands, |
| } else { | ||
| // `resolve_name_in_module` reported a privacy error. | ||
| self.import_dummy_binding(directive); | ||
| Success(()) |
There was a problem hiding this comment.
This error reporting system is correct but (imo) ugly.
After finishing #[feature(item_like_imports)], I plan on adding a name resolution error type (more expressive than today's (Span, String)) which will be able to represent privacy errors and ambiguity errors as well as arbitrary (Span, String) errors.
|
Question about the current_vis commit: why do you track this in resolve? Is it needed in resolve, or is it just to have that info for privacy checking? |
| type_determined: Cell<bool>, | ||
| value_determined: Cell<bool>, | ||
| value_result: Cell<Result<&'a NameBinding<'a>, bool /* determined? */>>, | ||
| type_result: Cell<Result<&'a NameBinding<'a>, bool /* determined? */>>, |
There was a problem hiding this comment.
Could you use an enum instead of these bools?
|
Looks good! A few comments inline, I guess I'm a bit uneasy about moving some of the privacy checking into resolution, but we do some already. Do you think it would be possible to separate further rather than merging? Or is that a dream? |
|
I believe we need to perform at least some privacy checking in mod foo {
pub mod bar {}
pub(some::path) fn bar() { println!("1"); }
}
mod baz {
pub fn bar() { println!("2"); }
}
fn main() {
use foo::bar; // Whether this imports the value depends on accessibility.
use baz::*;
bar(); // If `pub(some::path)` items are accessible here, this is "1"; otherwise, it is "2".
} |
jseyfried
force-pushed
the
more_groundwork
branch
from
a314b4f
to
a7d852a
Compare
|
ty::Visibility::Restricted({
let normal_module = self.get_nearest_normal_module_parent_or_self(module);
self.definitions.as_local_node_id(normal_module.def_id().unwrap()).unwrap()
})This PR uses it outside of just privacy checking to deduce import resolution failure. For example, mod foo {
use bar::f::*; // (1) Since this is not accessible in bar,
// (4) ^ Thus we are able to resolve this import.
pub fn f() {}
}
mod bar {
pub use foo::f; // (2) we can deduce that this fails in the type namespace,
pub use baz::*; // (3) so we know that `bar::f` will resolve to `baz::f` since it won't get shadowed.
}
mod baz {
pub mod f {}
}Without being able to deduce |
jseyfried
force-pushed
the
more_groundwork
branch
from
a7d852a
to
a5566ff
Compare
jseyfried
force-pushed
the
more_groundwork
branch
from
a5566ff
to
a6e8f3b
Compare
|
@bors: r+ |
|
📌 Commit a6e8f3b has been approved by |
resolve: More groundwork for `item_like_imports` (RFC 1560) r? @nrc
| @@ -16,5 +16,4 @@ fn test_send<S: Send>() {} | |||
|
|
|||
| pub fn main() { | |||
| test_send::<rand::ThreadRng>(); | |||
| //~^ ERROR : std::marker::Send` is not satisfied | |||
There was a problem hiding this comment.
Wasn't this kind of the point of the test?
There was a problem hiding this comment.
Yeah, thanks for pointing that out -- fixed in #35894.
r? @nrc