From 6990b89b2650d8263dad348173f4f729d6753360 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 28 Feb 2021 04:39:38 +0100 Subject: [PATCH] Restrict visibilities to the containing DefMap --- crates/hir_def/src/body/tests/block.rs | 29 +++++++++++++++++++ crates/hir_def/src/nameres/path_resolution.rs | 21 +++++++++++--- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/crates/hir_def/src/body/tests/block.rs b/crates/hir_def/src/body/tests/block.rs index a5ec0883fb8..8bca72a1759 100644 --- a/crates/hir_def/src/body/tests/block.rs +++ b/crates/hir_def/src/body/tests/block.rs @@ -259,3 +259,32 @@ mod module { "#]], ); } + +#[test] +fn underscore_import() { + // This used to panic, because the default (private) visibility inside block expressions would + // point into the containing `DefMap`, which visibilities should never be able to do. + mark::check!(adjust_vis_in_block_def_map); + check_at( + r#" +mod m { + fn main() { + use Tr as _; + trait Tr {} + $0 + } +} + "#, + expect![[r#" + block scope + _: t + Tr: t + + crate + m: t + + crate::m + main: v + "#]], + ); +} diff --git a/crates/hir_def/src/nameres/path_resolution.rs b/crates/hir_def/src/nameres/path_resolution.rs index fdcdc23ae73..dd1db0094b6 100644 --- a/crates/hir_def/src/nameres/path_resolution.rs +++ b/crates/hir_def/src/nameres/path_resolution.rs @@ -77,7 +77,7 @@ pub(crate) fn resolve_visibility( original_module: LocalModuleId, visibility: &RawVisibility, ) -> Option { - match visibility { + let mut vis = match visibility { RawVisibility::Module(path) => { let (result, remaining) = self.resolve_path(db, original_module, &path, BuiltinShadowMode::Module); @@ -86,15 +86,28 @@ pub(crate) fn resolve_visibility( } let types = result.take_types()?; match types { - ModuleDefId::ModuleId(m) => Some(Visibility::Module(m)), + ModuleDefId::ModuleId(m) => Visibility::Module(m), _ => { // error: visibility needs to refer to module - None + return None; } } } - RawVisibility::Public => Some(Visibility::Public), + RawVisibility::Public => Visibility::Public, + }; + + // In block expressions, `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. + if let Visibility::Module(m) = vis { + if self.block_id() != m.block { + mark::hit!(adjust_vis_in_block_def_map); + vis = Visibility::Module(self.module_id(self.root())); + log::debug!("visibility {:?} points outside DefMap, adjusting to {:?}", m, vis); + } } + + Some(vis) } // Returns Yes if we are sure that additions to `ItemMap` wouldn't change