Mirrors/rust-analyzer
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-analyzer synced 2024-12-27 21:43:37 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #14176 - lowr:fix/assoc-func-vis-in-local-impl, r=Veykril

Browse source 
Fix associated item visibility in block-local impls

Fixes #14046

When we're resolving visibility of block-local items...

> `self` normally refers to the containing non-block module, and `super` to its parent (etc.). However, visibilities must only refer to a module in the DefMap they're written in, so we restrict them when that happens. ([link])

 ...unless we're resolving visibility of associated items in block-local impls, because that impl is semantically "hoisted" to the nearest (non-block) module. With this PR, we skip the adjustment for such items.

Since visibility representation of those items is modified, this PR also adjusts visibility rendering in `HirDisplay`.

[link]: a6603fc21d/crates/hir-def/src/nameres/path_resolution.rs (L101-L103)
This commit is contained in:
bors 2023-03-01 12:40:55 +00:00
commit 32424d0aba
10 changed files with 282 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
crates/hir-def/src/lib.rs
View file

@ -128,7 +128,7 @@ impl ModuleId {
}
}
/// An ID of a module, **local** to a specific crate
/// An ID of a module, **local** to a `DefMap`.
pub type LocalModuleId = Idx<nameres::ModuleData>;
#[derive(Debug)]

18
crates/hir-def/src/nameres.rs
View file

@ -342,7 +342,7 @@ impl DefMap {
}
pub(crate) fn block_id(&self) -> Option<BlockId> {
self.block.as_ref().map(|block| block.block)
self.block.map(|block| block.block)
}
pub(crate) fn prelude(&self) -> Option<ModuleId> {
@ -354,7 +354,7 @@ impl DefMap {
}
pub fn module_id(&self, local_id: LocalModuleId) -> ModuleId {
let block = self.block.as_ref().map(|b| b.block);
let block = self.block.map(|b| b.block);
ModuleId { krate: self.krate, local_id, block }
}
@ -432,9 +432,9 @@ impl DefMap {
/// Returns the module containing `local_mod`, either the parent `mod`, or the module containing
/// the block, if `self` corresponds to a block expression.
pub fn containing_module(&self, local_mod: LocalModuleId) -> Option<ModuleId> {
match &self[local_mod].parent {
Some(parent) => Some(self.module_id(*parent)),
None => self.block.as_ref().map(|block| block.parent),
match self[local_mod].parent {
Some(parent) => Some(self.module_id(parent)),
None => self.block.map(|block| block.parent),
}
}
@ -444,11 +444,11 @@ impl DefMap {
let mut buf = String::new();
let mut arc;
let mut current_map = self;
while let Some(block) = &current_map.block {
while let Some(block) = current_map.block {
go(&mut buf, current_map, "block scope", current_map.root);
buf.push('\n');
arc = block.parent.def_map(db);
current_map = &*arc;
current_map = &arc;
}
go(&mut buf, current_map, "crate", current_map.root);
return buf;
@ -472,10 +472,10 @@ impl DefMap {
let mut buf = String::new();
let mut arc;
let mut current_map = self;
while let Some(block) = &current_map.block {
while let Some(block) = current_map.block {
format_to!(buf, "{:?} in {:?}\n", block.block, block.parent);
arc = block.parent.def_map(db);
current_map = &*arc;
current_map = &arc;
}
format_to!(buf, "crate scope\n");

12
crates/hir-def/src/nameres/collector.rs
View file

@ -666,8 +666,10 @@ impl DefCollector<'_> {
macro_: Macro2Id,
vis: &RawVisibility,
) {
let vis =
self.def_map.resolve_visibility(self.db, module_id, vis).unwrap_or(Visibility::Public);
let vis = self
.def_map
.resolve_visibility(self.db, module_id, vis, false)
.unwrap_or(Visibility::Public);
self.def_map.modules[module_id].scope.declare(macro_.into());
self.update(
module_id,
@ -831,7 +833,7 @@ impl DefCollector<'_> {
let mut def = directive.status.namespaces();
let vis = self
.def_map
.resolve_visibility(self.db, module_id, &directive.import.visibility)
.resolve_visibility(self.db, module_id, &directive.import.visibility, false)
.unwrap_or(Visibility::Public);
match import.kind {
@ -1547,7 +1549,7 @@ impl ModCollector<'_, '_> {
};
let resolve_vis = |def_map: &DefMap, visibility| {
def_map
.resolve_visibility(db, self.module_id, visibility)
.resolve_visibility(db, self.module_id, visibility, false)
.unwrap_or(Visibility::Public)
};
@ -1823,7 +1825,7 @@ impl ModCollector<'_, '_> {
) -> LocalModuleId {
let def_map = &mut self.def_collector.def_map;
let vis = def_map
.resolve_visibility(self.def_collector.db, self.module_id, visibility)
.resolve_visibility(self.def_collector.db, self.module_id, visibility, false)
.unwrap_or(Visibility::Public);
let modules = &mut def_map.modules;
let origin = match definition {

4
crates/hir-def/src/nameres/path_resolution.rs
View file

@ -78,6 +78,7 @@ impl DefMap {
// pub(path)
// ^^^^ this
visibility: &RawVisibility,
within_impl: bool,
) -> Option<Visibility> {
let mut vis = match visibility {
RawVisibility::Module(path) => {
@ -102,7 +103,8 @@ impl DefMap {
// `super` to its parent (etc.). However, visibilities must only refer to a module in the
// DefMap they're written in, so we restrict them when that happens.
if let Visibility::Module(m) = vis {
if self.block_id() != m.block {
// ...unless we're resolving visibility for an associated item in an impl.
if self.block_id() != m.block && !within_impl {
cov_mark::hit!(adjust_vis_in_block_def_map);
vis = Visibility::Module(self.module_id(self.root()));
tracing::debug!("visibility {:?} points outside DefMap, adjusting to {:?}", m, vis);

4
crates/hir-def/src/resolver.rs
View file

@ -214,10 +214,12 @@ impl Resolver {
db: &dyn DefDatabase,
visibility: &RawVisibility,
) -> Option<Visibility> {
let within_impl =
self.scopes().find(|scope| matches!(scope, Scope::ImplDefScope(_))).is_some();
match visibility {
RawVisibility::Module(_) => {
let (item_map, module) = self.item_scope();
item_map.resolve_visibility(db, module, visibility)
item_map.resolve_visibility(db, module, visibility, within_impl)
}
RawVisibility::Public => Some(Visibility::Public),
}

7
crates/hir-def/src/visibility.rs
View file

@ -11,7 +11,7 @@ use crate::{
nameres::DefMap,
path::{ModPath, PathKind},
resolver::HasResolver,
ConstId, FunctionId, HasModule, LocalFieldId, ModuleId, VariantId,
ConstId, FunctionId, HasModule, LocalFieldId, LocalModuleId, ModuleId, VariantId,
};
/// Visibility of an item, not yet resolved.
@ -120,7 +120,7 @@ impl Visibility {
self,
db: &dyn DefDatabase,
def_map: &DefMap,
mut from_module: crate::LocalModuleId,
mut from_module: LocalModuleId,
) -> bool {
let mut to_module = match self {
Visibility::Module(m) => m,
@ -142,7 +142,8 @@ impl Visibility {
arc = to_module.def_map(db);
&arc
};
let is_block_root = matches!(to_module.block, Some(_) if to_module_def_map[to_module.local_id].parent.is_none());
let is_block_root =
to_module.block.is_some() && to_module_def_map[to_module.local_id].parent.is_none();
if is_block_root {
to_module = to_module_def_map.containing_module(to_module.local_id).unwrap();
}

33
crates/hir/src/display.rs
View file

@ -17,15 +17,23 @@ use hir_ty::{
};
use crate::{
Adt, Const, ConstParam, Enum, Field, Function, GenericParam, HasCrate, HasVisibility,
LifetimeParam, Macro, Module, Static, Struct, Trait, TyBuilder, Type, TypeAlias,
TypeOrConstParam, TypeParam, Union, Variant,
Adt, AsAssocItem, AssocItemContainer, Const, ConstParam, Enum, Field, Function, GenericParam,
HasCrate, HasVisibility, LifetimeParam, Macro, Module, Static, Struct, Trait, TyBuilder, Type,
TypeAlias, TypeOrConstParam, TypeParam, Union, Variant,
};
impl HirDisplay for Function {
fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> {
let data = f.db.function_data(self.id);
write_visibility(self.module(f.db).id, self.visibility(f.db), f)?;
let db = f.db;
let data = db.function_data(self.id);
let container = self.as_assoc_item(db).map(|it| it.container(db));
let mut module = self.module(db);
if let Some(AssocItemContainer::Impl(_)) = container {
// Block-local impls are "hoisted" to the nearest (non-block) module.
module = module.nearest_non_block_module(db);
}
let module_id = module.id;
write_visibility(module_id, self.visibility(db), f)?;
if data.has_default_kw() {
f.write_str("default ")?;
}
@ -35,7 +43,7 @@ impl HirDisplay for Function {
if data.has_async_kw() {
f.write_str("async ")?;
}
if self.is_unsafe_to_call(f.db) {
if self.is_unsafe_to_call(db) {
f.write_str("unsafe ")?;
}
if let Some(abi) = &data.abi {
@ -50,7 +58,7 @@ impl HirDisplay for Function {
let write_self_param = |ty: &TypeRef, f: &mut HirFormatter<'_>| match ty {
TypeRef::Path(p) if p.is_self_type() => f.write_str("self"),
TypeRef::Reference(inner, lifetime, mut_) if matches!(&**inner,TypeRef::Path(p) if p.is_self_type()) =>
TypeRef::Reference(inner, lifetime, mut_) if matches!(&**inner, TypeRef::Path(p) if p.is_self_type()) =>
{
f.write_char('&')?;
if let Some(lifetime) = lifetime {
@ -442,8 +450,15 @@ fn write_where_clause(def: GenericDefId, f: &mut HirFormatter<'_>) -> Result<(),
impl HirDisplay for Const {
fn hir_fmt(&self, f: &mut HirFormatter<'_>) -> Result<(), HirDisplayError> {
write_visibility(self.module(f.db).id, self.visibility(f.db), f)?;
let data = f.db.const_data(self.id);
let db = f.db;
let container = self.as_assoc_item(db).map(|it| it.container(db));
let mut module = self.module(db);
if let Some(AssocItemContainer::Impl(_)) = container {
// Block-local impls are "hoisted" to the nearest (non-block) module.
module = module.nearest_non_block_module(db);
}
write_visibility(module.id, self.visibility(db), f)?;
let data = db.const_data(self.id);
f.write_str("const ")?;
match &data.name {
Some(name) => write!(f, "{name}: ")?,

16
crates/hir/src/lib.rs
View file

@ -46,7 +46,7 @@ use hir_def::{
item_tree::ItemTreeNode,
lang_item::{LangItem, LangItemTarget},
layout::{Layout, LayoutError, ReprOptions},
nameres::{self, diagnostics::DefDiagnostic},
nameres::{self, diagnostics::DefDiagnostic, ModuleOrigin},
per_ns::PerNs,
resolver::{HasResolver, Resolver},
src::HasSource as _,
@ -488,6 +488,20 @@ impl Module {
Some(Module { id: def_map.module_id(parent_id) })
}
/// Finds nearest non-block ancestor `Module` (`self` included).
fn nearest_non_block_module(self, db: &dyn HirDatabase) -> Module {
let mut id = self.id;
loop {
let def_map = id.def_map(db.upcast());
let origin = def_map[id.local_id].origin;
if matches!(origin, ModuleOrigin::BlockExpr { .. }) {
id = id.containing_module(db.upcast()).expect("block without parent module")
} else {
return Module { id };
}
}
}
pub fn path_to_root(self, db: &dyn HirDatabase) -> Vec<Module> {
let mut res = vec![self];
let mut curr = self;

38
crates/ide-diagnostics/src/handlers/private_assoc_item.rs
View file

@ -115,6 +115,44 @@ mod module {
fn main(s: module::Struct) {
s.method();
}
"#,
);
}
#[test]
fn can_see_through_top_level_anonymous_const() {
// regression test for #14046.
check_diagnostics(
r#"
struct S;
mod m {
const _: () = {
impl crate::S {
pub(crate) fn method(self) {}
pub(crate) const A: usize = 42;
}
};
mod inner {
const _: () = {
impl crate::S {
pub(crate) fn method2(self) {}
pub(crate) const B: usize = 42;
pub(super) fn private(self) {}
pub(super) const PRIVATE: usize = 42;
}
};
}
}
fn main() {
S.method();
S::A;
S.method2();
S::B;
S.private();
//^^^^^^^^^^^ error: function `private` is private
S::PRIVATE;
//^^^^^^^^^^ error: const `PRIVATE` is private
}
"#,
);
}

178
crates/ide/src/hover/tests.rs
View file

@ -5619,3 +5619,181 @@ fn main() {
"#]],
);
}
#[test]
fn assoc_fn_in_block_local_impl() {
check(
r#"
struct S;
mod m {
const _: () = {
impl crate::S {
pub(crate) fn foo() {}
}
};
}
fn test() {
S::foo$0();
}
"#,
expect![[r#"
*foo*
```rust
test::S
```
```rust
pub(crate) fn foo()
```
"#]],
);
check(
r#"
struct S;
mod m {
const _: () = {
const _: () = {
impl crate::S {
pub(crate) fn foo() {}
}
};
};
}
fn test() {
S::foo$0();
}
"#,
expect![[r#"
*foo*
```rust
test::S
```
```rust
pub(crate) fn foo()
```
"#]],
);
check(
r#"
struct S;
mod m {
mod inner {
const _: () = {
impl crate::S {
pub(super) fn foo() {}
}
};
}
fn test() {
crate::S::foo$0();
}
}
"#,
expect![[r#"
*foo*
```rust
test::S
```
```rust
pub(super) fn foo()
```
"#]],
);
}
#[test]
fn assoc_const_in_block_local_impl() {
check(
r#"
struct S;
mod m {
const _: () = {
impl crate::S {
pub(crate) const A: () = ();
}
};
}
fn test() {
S::A$0;
}
"#,
expect![[r#"
*A*
```rust
test
```
```rust
pub(crate) const A: () = ()
```
"#]],
);
check(
r#"
struct S;
mod m {
const _: () = {
const _: () = {
impl crate::S {
pub(crate) const A: () = ();
}
};
};
}
fn test() {
S::A$0;
}
"#,
expect![[r#"
*A*
```rust
test
```
```rust
pub(crate) const A: () = ()
```
"#]],
);
check(
r#"
struct S;
mod m {
mod inner {
const _: () = {
impl crate::S {
pub(super) const A: () = ();
}
};
}
fn test() {
crate::S::A$0;
}
}
"#,
expect![[r#"
*A*
```rust
test
```
```rust
pub(super) const A: () = ()
```
"#]],
);
}