From 8c5f95a30dcd4fb13458ef987f0f3be6f4a6bd49 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 21 Jul 2026 16:42:11 -0400 Subject: [PATCH 01/12] Qualified flux items names --- crates/flux-desugar/src/resolver.rs | 106 +++++++++++------- .../src/resolver/refinement_resolver.rs | 7 +- crates/flux-metadata/src/lib.rs | 33 ++++++ crates/flux-middle/src/cstore.rs | 3 +- crates/flux-middle/src/fhir.rs | 33 +++++- crates/flux-middle/src/global_env.rs | 6 + crates/flux-middle/src/queries.rs | 56 ++++++++- tests/tests/neg/surface/resolver00.rs | 15 +++ .../auxiliary/flux_mod_children_aux.rs | 7 ++ tests/tests/pos/surface/resolver04.rs | 59 ++++++++++ tests/tests/pos/surface/resolver05.rs | 9 ++ 11 files changed, 287 insertions(+), 47 deletions(-) create mode 100644 tests/tests/neg/surface/resolver00.rs create mode 100644 tests/tests/pos/surface/auxiliary/flux_mod_children_aux.rs create mode 100644 tests/tests/pos/surface/resolver04.rs create mode 100644 tests/tests/pos/surface/resolver05.rs diff --git a/crates/flux-desugar/src/resolver.rs b/crates/flux-desugar/src/resolver.rs index ec4db479347..85afe45fb87 100644 --- a/crates/flux-desugar/src/resolver.rs +++ b/crates/flux-desugar/src/resolver.rs @@ -111,45 +111,52 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } } + /// Qualifiers and primop-props are global #[allow(clippy::disallowed_methods, reason = "`flux_items_by_parent` is the source of truth")] fn define_flux_global_items(&mut self) { - // Note that names are defined globally so we check for duplicates globally in the crate. - let mut definitions = DefinitionMap::default(); for (parent, items) in &self.specs.flux_items_by_parent { for item in items { - // NOTE: This is putting all items in the same namespace. In principle, we could have - // qualifiers in a different namespace. - definitions - .define(item.name()) - .emit(&self.genv) - .collect_err(&mut self.err); - match item { surface::FluxItem::Qualifier(qual) => { let def_id = FluxLocalDefId::new(parent.def_id, qual.name.name); self.qualifiers.insert(qual.name.name, def_id); } - surface::FluxItem::FuncDef(defn) => { - let parent = parent.def_id.to_def_id(); - let name = defn.name.name; - let def_id = FluxDefId::new(parent, name); - let kind = fhir::SpecFuncKind::Def(def_id); - self.define_in_prelude(name, fhir::Res::GlobalFunc(kind), ReftNS); - } surface::FluxItem::PrimOpProp(primop_prop) => { - let name = primop_prop.name.name; - let parent = parent.def_id.to_def_id(); - let def_id = FluxDefId::new(parent, name); - self.primop_props.insert(name, def_id); - } - surface::FluxItem::SortDecl(sort_decl) => { - let def_id = FluxDefId::new(parent.def_id.to_def_id(), sort_decl.name.name); - self.define_in_prelude( - sort_decl.name.name, - fhir::Res::UserSort(def_id), - TypeNS, - ); + let def_id = + FluxDefId::new(parent.def_id.to_def_id(), primop_prop.name.name); + self.primop_props.insert(primop_prop.name.name, def_id); } + surface::FluxItem::FuncDef(_) | surface::FluxItem::SortDecl(_) => {} + } + } + } + } + + #[allow(clippy::disallowed_methods, reason = "`flux_items_by_parent` is the source of truth")] + fn define_module_flux_items(&mut self, parent: OwnerId) { + let Some(items) = self.specs.flux_items_by_parent.get(&parent) else { return }; + // Names are defined per-module so duplicates are only checked within this module. + let mut definitions = DefinitionMap::default(); + for item in items { + // NOTE: This is putting all items in the same namespace. In principle, we could have + // qualifiers in a different namespace. + definitions + .define(item.name()) + .emit(&self.genv) + .collect_err(&mut self.err); + + match item { + // Already registered in `define_global_qualifiers_and_primop_props`. + surface::FluxItem::Qualifier(_) | surface::FluxItem::PrimOpProp(_) => {} + surface::FluxItem::FuncDef(defn) => { + let name = defn.name.name; + let def_id = FluxDefId::new(parent.def_id.to_def_id(), name); + let kind = fhir::SpecFuncKind::Def(def_id); + self.define_res_in(name, fhir::Res::GlobalFunc(kind), ReftNS); + } + surface::FluxItem::SortDecl(sort_decl) => { + let def_id = FluxDefId::new(parent.def_id.to_def_id(), sort_decl.name.name); + self.define_res_in(sort_decl.name.name, fhir::Res::UserSort(def_id), TypeNS); } } } @@ -430,16 +437,30 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { let tcx = self.genv.tcx(); match module.kind { ModuleKind::Mod => { - // Module children are Rust items, so we only ever resolve them in a Rust namespace. - let rustc_ns = ns.to_rustc()?; let module_id = module.def_id; let current_mod = self.current_module.to_def_id(); - visible_module_children(tcx, module_id, current_mod) - .find(|child| { - child.res.matches_ns(rustc_ns) - && tcx.hygienic_eq(ident, child.ident, current_mod) + // Rust module children take precedence, but are only resolved in a Rust + // namespace. + ns.to_rustc() + .and_then(|rustc_ns| { + visible_module_children(tcx, module_id, current_mod) + .find(|child| { + child.res.matches_ns(rustc_ns) + && tcx.hygienic_eq(ident, child.ident, current_mod) + }) + .and_then(|child| { + fhir::Res::::try_from(child.res).ok() + }) + }) + .or_else(|| { + self.genv + .flux_module_children(module_id) + .iter() + .find(|child| { + child.res.ns() == Some(ns) && child.ident.name == ident.name + }) + .map(|child| child.res.map_param_id(|id| match id {})) }) - .and_then(|child| fhir::Res::::try_from(child.res).ok()) } ModuleKind::Trait => { // Associated items are Rust items, so we only ever resolve them in a Rust namespace. @@ -486,36 +507,43 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { self.current_module = hir_id.expect_owner(); self.push_rib(TypeNS, RibKind::Module); self.push_rib(ValueNS, RibKind::Module); + self.push_rib(ReftNS, RibKind::Module); self.define_items(module.item_ids); - // Flux items are made globally available as if they were defined at the top of the crate + // Flux primops and wualifiers are made globally available as if they were defined at the top of the crate if hir_id == CRATE_HIR_ID { self.define_flux_global_items(); } + // Other items are defined in the module they are declared in. + self.define_module_flux_items(hir_id.expect_owner()); - // But we resolve names in them as if they were defined in their containing module self.resolve_flux_items(hir_id.expect_owner()); - hir::intravisit::walk_mod(self, module); + self.pop_rib(ReftNS); self.pop_rib(ValueNS); self.pop_rib(TypeNS); self.current_module = old_mod; } fn visit_block(&mut self, block: &'tcx hir::Block<'tcx>) { + let parent = self.genv.tcx().hir_get_parent_item(block.hir_id); + self.push_rib(TypeNS, RibKind::Misc); self.push_rib(ValueNS, RibKind::Misc); + self.push_rib(ReftNS, RibKind::Misc); let item_ids = block.stmts.iter().filter_map(|stmt| { if let hir::StmtKind::Item(item_id) = &stmt.kind { Some(item_id) } else { None } }); self.define_items(item_ids); - self.resolve_flux_items(self.genv.tcx().hir_get_parent_item(block.hir_id)); + self.define_module_flux_items(parent); + self.resolve_flux_items(parent); hir::intravisit::walk_block(self, block); + self.pop_rib(ReftNS); self.pop_rib(ValueNS); self.pop_rib(TypeNS); } diff --git a/crates/flux-desugar/src/resolver/refinement_resolver.rs b/crates/flux-desugar/src/resolver/refinement_resolver.rs index 76053b772c1..a157d83d336 100644 --- a/crates/flux-desugar/src/resolver/refinement_resolver.rs +++ b/crates/flux-desugar/src/resolver/refinement_resolver.rs @@ -637,7 +637,12 @@ impl ScopedVisitor for RefinementResolver<'_, '_, '_> { } fn on_refine_param(&mut self, param: &surface::RefineParam) { - self.define_param(param.ident, fhir::ParamKind::Explicit(param.mode), param.node_id, None); + self.define_param( + param.ident, + fhir::ParamKind::Explicit(param.mode.map(Into::into)), + param.node_id, + None, + ); } fn on_loc(&mut self, loc: Ident, node_id: NodeId) { diff --git a/crates/flux-metadata/src/lib.rs b/crates/flux-metadata/src/lib.rs index fb95054d952..45f84ea73ba 100644 --- a/crates/flux-metadata/src/lib.rs +++ b/crates/flux-metadata/src/lib.rs @@ -176,6 +176,7 @@ pub struct Tables<'tcx, K: Eq + Hash> { variants_of: UnordMap>>>, type_of: UnordMap>>, normalized_defns: Rc, + flux_module_children: UnordMap>, func_sort: UnordMap, rty::PolyFuncSort>, func_span: UnordMap, Span>, sort_decl_param_count: UnordMap, usize>, @@ -250,6 +251,19 @@ macro_rules! get { }}; } +/// Same as `get!` but returns a reference into the tables instead of cloning +macro_rules! get_ref { + ($self:expr, $table:ident, $key:expr) => {{ + let key = $key; + let this = $self; + if let Some(tables) = this.local_tables.get(&key.crate_num()) { + tables.$table.get(&key.to_index()) + } else { + this.extern_tables.$table.get(&key) + } + }}; +} + impl<'tcx> CrateStore<'tcx> for CStore<'tcx> { fn fn_sig(&self, def_id: DefId) -> OptResult> { get!(self, fn_sig, def_id) @@ -332,6 +346,10 @@ impl<'tcx> CrateStore<'tcx> for CStore<'tcx> { self.local_tables[&krate].normalized_defns.clone() } + fn flux_module_children(&self, def_id: DefId) -> Option<&[fhir::FluxModChild]> { + get_ref!(self, flux_module_children, def_id).map(Vec::as_slice) + } + fn inferred_no_panic(&self, krate: CrateNum) -> Rc, PanicSpec>> { // TODO: Some transitive deps (e.g. `hashbrown`) have no flux metadata. Return // an empty map (conservative: MightPanic) until the proper fix is in place. @@ -407,6 +425,7 @@ impl<'tcx> CrateMetadata<'tcx> { fn encode_flux_defs<'tcx>(genv: GlobalEnv<'_, 'tcx>, tables: &mut Tables<'tcx, DefIndex>) { tables.normalized_defns = genv.normalized_defns(LOCAL_CRATE); + encode_flux_module_children(genv, tables); for (def_id, item) in genv.fhir_iter_flux_items() { match item { fhir::FluxItem::Func(spec_func) => { @@ -427,6 +446,20 @@ fn encode_flux_defs<'tcx>(genv: GlobalEnv<'_, 'tcx>, tables: &mut Tables<'tcx, D } } +#[allow(clippy::disallowed_methods, reason = "`flux_items_by_parent` is the source of truth")] +fn encode_flux_module_children<'tcx>( + genv: GlobalEnv<'_, 'tcx>, + tables: &mut Tables<'tcx, DefIndex>, +) { + let specs = genv.collect_specs(); + for parent in specs.flux_items_by_parent.keys() { + let children = genv.flux_module_children(parent.def_id.to_def_id()); + tables + .flux_module_children + .insert(parent.def_id.local_def_index, children.to_vec()); + } +} + fn encode_def_ids<'tcx, K: Eq + Hash + Copy>( genv: GlobalEnv<'_, 'tcx>, def_ids: impl IntoIterator, diff --git a/crates/flux-middle/src/cstore.rs b/crates/flux-middle/src/cstore.rs index 557778fcca2..6da7793fa46 100644 --- a/crates/flux-middle/src/cstore.rs +++ b/crates/flux-middle/src/cstore.rs @@ -4,7 +4,7 @@ use rustc_data_structures::unord::{UnordMap, UnordSet}; use rustc_hir::def_id::CrateNum; use rustc_span::{Span, def_id::DefId}; -use crate::{PanicSpec, call_graph::NodeKey, def_id::FluxDefId, queries::QueryResult, rty}; +use crate::{PanicSpec, call_graph::NodeKey, def_id::FluxDefId, fhir, queries::QueryResult, rty}; pub type OptResult = Option>; @@ -34,6 +34,7 @@ pub trait CrateStore<'tcx> { ) -> OptResult>>; fn type_of(&self, def_id: DefId) -> OptResult>; fn normalized_defns(&self, krate: CrateNum) -> Rc; + fn flux_module_children(&self, def_id: DefId) -> Option<&[fhir::FluxModChild]>; fn func_sort(&self, def_id: FluxDefId) -> Option; fn func_span(&self, def_id: FluxDefId) -> Option; fn sort_decl_param_count(&self, def_id: FluxDefId) -> Option; diff --git a/crates/flux-middle/src/fhir.rs b/crates/flux-middle/src/fhir.rs index d3c2021c4b9..dad71f7a9bd 100644 --- a/crates/flux-middle/src/fhir.rs +++ b/crates/flux-middle/src/fhir.rs @@ -21,7 +21,7 @@ use flux_common::{bug, span_bug}; use flux_config::PartialInferOpts; pub use flux_syntax::surface::{BinOp, UnOp}; use flux_syntax::{ - surface::{Ignored, ParamMode, Trusted}, + surface::{self, Ignored, Trusted}, symbols::sym, }; use itertools::Itertools; @@ -835,7 +835,7 @@ impl std::ops::IndexMut for PerNS { /// /// The enum contains a subset of the variants in [`rustc_hir::def::Res`] plus some extra variants /// for stuff refinements resolve to. -#[derive(Eq, PartialEq, Debug, Copy, Clone)] +#[derive(Eq, PartialEq, Debug, Copy, Clone, Encodable, Decodable)] pub enum Res { /// See [`rustc_hir::def::Res::Def`] Def(DefKind, DefId), @@ -863,6 +863,13 @@ pub enum Res { Err, } +/// Akin to `rustc_middle::metadata::ModChild` but for flux items defined in a module +#[derive(Debug, Clone, Copy, Encodable, Decodable)] +pub struct FluxModChild { + pub ident: Ident, + pub res: Res, +} + /// See [`rustc_hir::def::PartialRes`] #[derive(Copy, Clone, Debug)] pub struct PartialRes { @@ -920,8 +927,23 @@ pub struct RefineParam<'fhir> { pub fhir_id: FhirId, } +#[derive(Clone, Copy, Debug, PartialEq, Eq, Encodable, Decodable)] +pub enum ParamMode { + Horn, + Hindley, +} + +impl From for ParamMode { + fn from(value: surface::ParamMode) -> Self { + match value { + surface::ParamMode::Horn => Self::Horn, + surface::ParamMode::Hindley => Self::Hindley, + } + } +} + /// How a parameter was declared in the surface syntax. -#[derive(PartialEq, Eq, Debug, Clone, Copy)] +#[derive(PartialEq, Eq, Debug, Clone, Copy, Encodable, Decodable)] pub enum ParamKind { /// A parameter declared in an explicit scope, e.g., `fn foo[hdl n: int](x: i32[n])` Explicit(Option), @@ -985,7 +1007,7 @@ impl InferMode { /// `bool`, `char`, and `str` are primitive sorts, but because sorts and types are in the same /// namespace we resolve them to [`Res::PrimTy`] and then make them into a sort during `conv` /// they share their name with the -#[derive(Debug, Eq, PartialEq, Clone, Copy)] +#[derive(Debug, Eq, PartialEq, Clone, Copy, Encodable, Decodable)] pub enum PrimSort { Int, Real, @@ -1199,6 +1221,7 @@ impl<'fhir> PathExpr<'fhir> { newtype_index! { #[debug_format = "a{}"] + #[encodable] pub struct ParamId {} } @@ -1364,7 +1387,7 @@ pub struct PrimOpProp<'fhir> { pub span: Span, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Encodable, Decodable)] pub enum SpecFuncKind { /// Theory symbols *interpreted* by the SMT solver Thy(liquid_fixpoint::ThyFunc), diff --git a/crates/flux-middle/src/global_env.rs b/crates/flux-middle/src/global_env.rs index 640061553aa..b704647cf2a 100644 --- a/crates/flux-middle/src/global_env.rs +++ b/crates/flux-middle/src/global_env.rs @@ -130,6 +130,12 @@ impl<'genv, 'tcx> GlobalEnv<'genv, 'tcx> { self.inner.queries.resolve_crate(self) } + /// Akin to `rustc_middle::ty::TyCtxt::module_children` but for flux items (`defs!` and + /// sort declarations) defined directly in a module. + pub fn flux_module_children(self, def_id: DefId) -> &'genv [fhir::FluxModChild] { + self.inner.queries.flux_module_children(self, def_id) + } + /// Parent directory of the Lean project. pub fn lean_parent_dir(self) -> PathBuf { lean_parent_dir(self.tcx()) diff --git a/crates/flux-middle/src/queries.rs b/crates/flux-middle/src/queries.rs index 27a59df6166..ee0ae57013b 100644 --- a/crates/flux-middle/src/queries.rs +++ b/crates/flux-middle/src/queries.rs @@ -13,7 +13,7 @@ use flux_rustc_bridge::{ mir::{self}, ty, }; -use flux_syntax::symbols::sym; +use flux_syntax::{surface, symbols::sym}; use itertools::Itertools; use rustc_data_structures::unord::{ExtendUnord, UnordMap, UnordSet}; use rustc_errors::Diagnostic; @@ -261,6 +261,7 @@ pub struct Queries<'genv, 'tcx> { mir: Cache>>>, collect_specs: OnceCell, resolve_crate: OnceCell, + flux_module_children: Cache, desugar: Cache>>, fhir_attr_map: Cache>, fhir_crate: OnceCell>, @@ -306,6 +307,7 @@ impl<'genv, 'tcx> Queries<'genv, 'tcx> { mir: Default::default(), collect_specs: Default::default(), resolve_crate: Default::default(), + flux_module_children: Default::default(), desugar: Default::default(), fhir_attr_map: Default::default(), fhir_crate: Default::default(), @@ -372,6 +374,58 @@ impl<'genv, 'tcx> Queries<'genv, 'tcx> { .get_or_init(|| (self.providers.resolve_crate)(genv)) } + /// Akin to `rustc_middle::ty::TyCtxt::module_children` but for flux items (`defs!` and + /// sort declarations) defined directly in a module. + #[allow(clippy::disallowed_methods, reason = "`flux_items_by_parent` is the source of truth")] + pub(crate) fn flux_module_children( + &'genv self, + genv: GlobalEnv<'genv, 'tcx>, + def_id: DefId, + ) -> &'genv [fhir::FluxModChild] { + run_with_cache(&self.flux_module_children, def_id, || { + def_id.dispatch_query( + genv, + self, + |def_id| -> &'genv [fhir::FluxModChild] { + // Local modules: build children from surface specs (safe to call from the + // resolver; `fhir_crate` would create a query cycle). Modules cannot + // have extern specs, so `local_id()` is sound. + let specs = genv.collect_specs(); + let parent = def_id.local_id(); + let items = specs + .flux_items_by_parent + .get(&rustc_hir::OwnerId { def_id: parent }) + .map_or(&[][..], Vec::as_ref); + genv.alloc_slice( + &items + .iter() + .filter_map(|item| { + let res = match item { + surface::FluxItem::FuncDef(_) => { + fhir::Res::GlobalFunc(fhir::SpecFuncKind::Def( + FluxDefId::new(parent.to_def_id(), item.name().name), + )) + } + surface::FluxItem::SortDecl(_) => { + fhir::Res::UserSort(FluxDefId::new( + parent.to_def_id(), + item.name().name, + )) + } + surface::FluxItem::Qualifier(_) + | surface::FluxItem::PrimOpProp(_) => return None, + }; + Some(fhir::FluxModChild { ident: item.name(), res }) + }) + .collect::>(), + ) + }, + |def_id| genv.cstore().flux_module_children(def_id), + |_| &[], // crates without flux metadata have no flux children + ) + }) + } + pub(crate) fn desugar( &'genv self, genv: GlobalEnv<'genv, 'tcx>, diff --git a/tests/tests/neg/surface/resolver00.rs b/tests/tests/neg/surface/resolver00.rs new file mode 100644 index 00000000000..8b199934953 --- /dev/null +++ b/tests/tests/neg/surface/resolver00.rs @@ -0,0 +1,15 @@ +mod mod_a { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 1 } + } +} + +mod mod_b { + // `shift` is only defined in `mod_a`, so it cannot be referred to from `mod_b` + #[flux::sig(fn(x: i32) -> i32[mod_b::shift(x)])] //~ ERROR cannot find value + pub fn test(x: i32) -> i32 { + x + 1 + } +} diff --git a/tests/tests/pos/surface/auxiliary/flux_mod_children_aux.rs b/tests/tests/pos/surface/auxiliary/flux_mod_children_aux.rs new file mode 100644 index 00000000000..c100be7295e --- /dev/null +++ b/tests/tests/pos/surface/auxiliary/flux_mod_children_aux.rs @@ -0,0 +1,7 @@ +pub mod mod_a { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 1 } + } +} diff --git a/tests/tests/pos/surface/resolver04.rs b/tests/tests/pos/surface/resolver04.rs new file mode 100644 index 00000000000..f7d740ef40f --- /dev/null +++ b/tests/tests/pos/surface/resolver04.rs @@ -0,0 +1,59 @@ +//! Test that flux definitions can be referred to with qualified paths +#![allow(dead_code)] + +use flux_attrs::*; + +defs! { + fn inc_int(x: int) -> int { x + 1 } +} + +mod mod_a { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 1 } + + opaque sort Bag; + } + + // defs resolve unqualified inside their own module + #[sig(fn(x: i32) -> i32[shift(x)])] + pub fn test_inner(x: i32) -> i32 { + x + 1 + } +} + +// def in `mod_a` used with a qualified path from the crate root +#[flux::sig(fn(x: i32) -> i32[mod_a::shift(x)])] +pub fn test_mod_path(x: i32) -> i32 { + x + 1 +} + +// user sort in `mod_a` used with a qualified path +#[opaque] +#[refined_by(b: mod_a::Bag)] +pub struct WithSort { + inner: Vec, +} + +mod nested { + use flux_attrs::*; + + defs! { + fn dbl_int(x: int) -> int { 2 * x } + } + + // def at the crate root used with a qualified path from a nested module + #[flux::sig(fn(x: i32) -> i32[crate::inc_int(x)])] + pub fn test_crate_path(x: i32) -> i32 { + x + 1 + } + + mod inner { + // def in the parent module used with a qualified `super` path + #[flux::sig(fn(x: i32) -> i32[super::dbl_int(x)])] + pub fn test_super_path(x: i32) -> i32 { + 2 * x + } + } +} diff --git a/tests/tests/pos/surface/resolver05.rs b/tests/tests/pos/surface/resolver05.rs new file mode 100644 index 00000000000..c98a0e321a9 --- /dev/null +++ b/tests/tests/pos/surface/resolver05.rs @@ -0,0 +1,9 @@ +//@aux-build:flux_mod_children_aux.rs + +extern crate flux_mod_children_aux; + +// def in another crate used with a qualified path +#[flux::sig(fn(x: i32) -> i32[flux_mod_children_aux::mod_a::shift(x)])] +pub fn test(x: i32) -> i32 { + x + 1 +} From f3ebf41b86e75fb3e448b4acb98e39daf928112a Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Mon, 27 Jul 2026 15:49:53 -0400 Subject: [PATCH 02/12] tests --- tests/tests/pos/surface/resolver04.rs | 62 +++---------------- tests/tests/pos/surface/resolver05.rs | 60 ++++++++++++++++-- tests/tests/pos/surface/resolver06.rs | 9 +++ .../tests/with_deps/pos/surface/resolver04.rs | 15 ----- 4 files changed, 73 insertions(+), 73 deletions(-) create mode 100644 tests/tests/pos/surface/resolver06.rs delete mode 100644 tests/tests/with_deps/pos/surface/resolver04.rs diff --git a/tests/tests/pos/surface/resolver04.rs b/tests/tests/pos/surface/resolver04.rs index f7d740ef40f..afcdbce2204 100644 --- a/tests/tests/pos/surface/resolver04.rs +++ b/tests/tests/pos/surface/resolver04.rs @@ -1,59 +1,15 @@ -//! Test that flux definitions can be referred to with qualified paths -#![allow(dead_code)] +// Test that we support `super` in name resolution -use flux_attrs::*; +struct S; -defs! { - fn inc_int(x: int) -> int { x + 1 } -} - -mod mod_a { - use flux_attrs::*; - - defs! { - fn shift(x: int) -> int { x + 1 } - - opaque sort Bag; - } - - // defs resolve unqualified inside their own module - #[sig(fn(x: i32) -> i32[shift(x)])] - pub fn test_inner(x: i32) -> i32 { - x + 1 - } -} +mod a { + use super::*; -// def in `mod_a` used with a qualified path from the crate root -#[flux::sig(fn(x: i32) -> i32[mod_a::shift(x)])] -pub fn test_mod_path(x: i32) -> i32 { - x + 1 + #[flux_attrs::spec(fn(S))] + fn foo(s: S) {} } -// user sort in `mod_a` used with a qualified path -#[opaque] -#[refined_by(b: mod_a::Bag)] -pub struct WithSort { - inner: Vec, -} - -mod nested { - use flux_attrs::*; - - defs! { - fn dbl_int(x: int) -> int { 2 * x } - } - - // def at the crate root used with a qualified path from a nested module - #[flux::sig(fn(x: i32) -> i32[crate::inc_int(x)])] - pub fn test_crate_path(x: i32) -> i32 { - x + 1 - } - - mod inner { - // def in the parent module used with a qualified `super` path - #[flux::sig(fn(x: i32) -> i32[super::dbl_int(x)])] - pub fn test_super_path(x: i32) -> i32 { - 2 * x - } - } +mod b { + #[flux_attrs::spec(fn(super::S))] + fn foo(s: super::S) {} } diff --git a/tests/tests/pos/surface/resolver05.rs b/tests/tests/pos/surface/resolver05.rs index c98a0e321a9..f7d740ef40f 100644 --- a/tests/tests/pos/surface/resolver05.rs +++ b/tests/tests/pos/surface/resolver05.rs @@ -1,9 +1,59 @@ -//@aux-build:flux_mod_children_aux.rs +//! Test that flux definitions can be referred to with qualified paths +#![allow(dead_code)] -extern crate flux_mod_children_aux; +use flux_attrs::*; -// def in another crate used with a qualified path -#[flux::sig(fn(x: i32) -> i32[flux_mod_children_aux::mod_a::shift(x)])] -pub fn test(x: i32) -> i32 { +defs! { + fn inc_int(x: int) -> int { x + 1 } +} + +mod mod_a { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 1 } + + opaque sort Bag; + } + + // defs resolve unqualified inside their own module + #[sig(fn(x: i32) -> i32[shift(x)])] + pub fn test_inner(x: i32) -> i32 { + x + 1 + } +} + +// def in `mod_a` used with a qualified path from the crate root +#[flux::sig(fn(x: i32) -> i32[mod_a::shift(x)])] +pub fn test_mod_path(x: i32) -> i32 { x + 1 } + +// user sort in `mod_a` used with a qualified path +#[opaque] +#[refined_by(b: mod_a::Bag)] +pub struct WithSort { + inner: Vec, +} + +mod nested { + use flux_attrs::*; + + defs! { + fn dbl_int(x: int) -> int { 2 * x } + } + + // def at the crate root used with a qualified path from a nested module + #[flux::sig(fn(x: i32) -> i32[crate::inc_int(x)])] + pub fn test_crate_path(x: i32) -> i32 { + x + 1 + } + + mod inner { + // def in the parent module used with a qualified `super` path + #[flux::sig(fn(x: i32) -> i32[super::dbl_int(x)])] + pub fn test_super_path(x: i32) -> i32 { + 2 * x + } + } +} diff --git a/tests/tests/pos/surface/resolver06.rs b/tests/tests/pos/surface/resolver06.rs new file mode 100644 index 00000000000..c98a0e321a9 --- /dev/null +++ b/tests/tests/pos/surface/resolver06.rs @@ -0,0 +1,9 @@ +//@aux-build:flux_mod_children_aux.rs + +extern crate flux_mod_children_aux; + +// def in another crate used with a qualified path +#[flux::sig(fn(x: i32) -> i32[flux_mod_children_aux::mod_a::shift(x)])] +pub fn test(x: i32) -> i32 { + x + 1 +} diff --git a/tests/tests/with_deps/pos/surface/resolver04.rs b/tests/tests/with_deps/pos/surface/resolver04.rs deleted file mode 100644 index 03ce80eadff..00000000000 --- a/tests/tests/with_deps/pos/surface/resolver04.rs +++ /dev/null @@ -1,15 +0,0 @@ -// Test that we support `super` in name resolution - -struct S; - -mod a { - use super::*; - - #[flux_rs::spec(fn(S))] - fn foo(s: S) {} -} - -mod b { - #[flux_rs::spec(fn(super::S))] - fn foo(s: super::S) {} -} From e87142de91522a036aade1c6d7b5701f1018cc97 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 28 Jul 2026 13:21:27 -0400 Subject: [PATCH 03/12] Implement use statements for flux items --- Cargo.lock | 1 + crates/flux-desugar/src/desugar.rs | 1 + crates/flux-desugar/src/lib.rs | 3 +- crates/flux-desugar/src/resolver.rs | 128 ++++++++++++++---- .../src/resolver/refinement_resolver.rs | 6 +- crates/flux-middle/src/queries.rs | 27 ++-- crates/flux-syntax/Cargo.toml | 2 + crates/flux-syntax/src/parser/mod.rs | 13 ++ crates/flux-syntax/src/surface.rs | 20 ++- crates/flux-syntax/src/surface/visit.rs | 1 + 10 files changed, 161 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ef7c0d1e2dd..c83034541f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -718,6 +718,7 @@ version = "0.1.0" dependencies = [ "flux-config", "flux-macros", + "itertools 0.14.0", "rustc-hash", ] diff --git a/crates/flux-desugar/src/desugar.rs b/crates/flux-desugar/src/desugar.rs index 8e713dd5a6d..3717a679456 100644 --- a/crates/flux-desugar/src/desugar.rs +++ b/crates/flux-desugar/src/desugar.rs @@ -852,6 +852,7 @@ impl<'genv, 'tcx> FluxItemCtxt<'genv, 'tcx> { let sort_decl = self.desugar_sort_decl(sort_decl); fhir::FluxItem::SortDecl(self.genv.alloc(sort_decl)) } + surface::FluxItem::Use(..) => bug!("unexpected use item"), } } diff --git a/crates/flux-desugar/src/lib.rs b/crates/flux-desugar/src/lib.rs index 99296f5febc..6be225559e6 100644 --- a/crates/flux-desugar/src/lib.rs +++ b/crates/flux-desugar/src/lib.rs @@ -162,7 +162,8 @@ fn try_desugar_crate<'genv>(genv: GlobalEnv<'genv, '_>) -> Result = None; for (parent, items) in &specs.flux_items_by_parent { for item in items { - let def_id = FluxLocalDefId::new(parent.def_id, item.name().name); + let Some(ident) = item.name() else { continue }; + let def_id = FluxLocalDefId::new(parent.def_id, ident.name); FluxItemCtxt::with(genv, resolver_output, def_id, |cx| { fhir.items.insert(def_id, cx.desugar_flux_item(item)); }) diff --git a/crates/flux-desugar/src/resolver.rs b/crates/flux-desugar/src/resolver.rs index 85afe45fb87..1918d089661 100644 --- a/crates/flux-desugar/src/resolver.rs +++ b/crates/flux-desugar/src/resolver.rs @@ -2,7 +2,10 @@ pub(crate) mod refinement_resolver; use std::collections::hash_map; -use flux_common::result::{ErrorCollector, ResultExt}; +use flux_common::{ + bug, + result::{ErrorCollector, ResultExt}, +}; use flux_errors::Errors; use flux_middle::{ ResolverOutput, Specs, @@ -112,7 +115,10 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } /// Qualifiers and primop-props are global - #[allow(clippy::disallowed_methods, reason = "`flux_items_by_parent` is the source of truth")] + #[allow( + clippy::disallowed_methods, + reason = "`flux_items_by_parent` is the source of truth for `FluxDefId`" + )] fn define_flux_global_items(&mut self) { for (parent, items) in &self.specs.flux_items_by_parent { for item in items { @@ -126,24 +132,30 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { FluxDefId::new(parent.def_id.to_def_id(), primop_prop.name.name); self.primop_props.insert(primop_prop.name.name, def_id); } - surface::FluxItem::FuncDef(_) | surface::FluxItem::SortDecl(_) => {} + surface::FluxItem::Use(_) + | surface::FluxItem::FuncDef(_) + | surface::FluxItem::SortDecl(_) => {} } } } } - #[allow(clippy::disallowed_methods, reason = "`flux_items_by_parent` is the source of truth")] + #[allow( + clippy::disallowed_methods, + reason = "`flux_items_by_parent` is the source of truth for `FluxDefId`" + )] fn define_module_flux_items(&mut self, parent: OwnerId) { let Some(items) = self.specs.flux_items_by_parent.get(&parent) else { return }; // Names are defined per-module so duplicates are only checked within this module. - let mut definitions = DefinitionMap::default(); + // let mut definitions = DefinitionMap::default(); for item in items { - // NOTE: This is putting all items in the same namespace. In principle, we could have - // qualifiers in a different namespace. - definitions - .define(item.name()) - .emit(&self.genv) - .collect_err(&mut self.err); + // let Some(ident) = item.name() else { continue }; + // // NOTE: This is putting all items in the same namespace. In principle, we could have + // // qualifiers in a different namespace. + // definitions + // .define(ident) + // .emit(&self.genv) + // .collect_err(&mut self.err); match item { // Already registered in `define_global_qualifiers_and_primop_props`. @@ -158,6 +170,11 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { let def_id = FluxDefId::new(parent.def_id.to_def_id(), sort_decl.name.name); self.define_res_in(sort_decl.name.name, fhir::Res::UserSort(def_id), TypeNS); } + surface::FluxItem::Use(path) => { + for (ident, res, ns) in self.resolve_flux_use_path(path) { + self.define_res_in(ident.name, res, ns); + } + } } } } @@ -328,7 +345,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { let is_last = segment_idx + 1 == segments.len(); let ns = if is_last { ns } else { TypeNS }; - let base_res = if let Some(module) = &module { + let base_res = if let Some(module) = module { self.resolve_ident_in_module(module, segment.ident(), ns)? } else { self.resolve_ident_with_ribs(segment.ident(), ns)? @@ -361,6 +378,71 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { None } + fn resolve_flux_use_path( + &mut self, + path: &surface::ExprPath, + ) -> Vec<(Ident, fhir::Res, Namespace)> { + use fhir::Res; + let [prefix @ .., last] = &path.segments[..] else { + bug!("path must have at least one segment") + }; + + // 1. Resolve prefix + let mut module_id: Option = None; + for segment in prefix { + let res = if let Some(module_id) = module_id { + let module = Module::new(ModuleKind::Mod, module_id); + self.resolve_ident_in_module(module, segment.ident(), TypeNS) + } else { + self.resolve_ident_with_ribs(segment.ident(), TypeNS) + }; + let Some(res) = res else { + self.emit(errors::UnresolvedName { + span: segment.ident().span, + name: segment.ident().to_string(), + kind: "import", + }); + return vec![]; + }; + + if let Res::Def(DefKind::Mod, def_id) = res { + module_id = Some(def_id); + } else { + self.emit(errors::UnresolvedName { + span: segment.ident().span, + name: segment.ident().to_string(), + kind: "import", + }); + return vec![]; + } + } + + // 2. Resolve last ident in all namespaces + let mut resolutions = vec![]; + for ns in [TypeNS, ValueNS, ReftNS] { + let res = if let Some(module_id) = module_id { + let module = Module::new(ModuleKind::Mod, module_id); + self.resolve_ident_in_module(module, last.ident(), ns) + } else { + self.resolve_ident_with_ribs(last.ident(), ns) + }; + if let Some(res) = res { + resolutions.push((last.ident(), res, ns)); + } + } + + // 3. Report error if no valid resolution + if resolutions.is_empty() { + self.emit(errors::UnresolvedName { + span: path.span, + name: path.display(), + kind: "import", + }) + } + + resolutions + } + fn resolve_ident_with_ribs( &self, ident: Ident, @@ -430,7 +512,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { fn resolve_ident_in_module( &self, - module: &Module, + module: Module, ident: Ident, ns: Namespace, ) -> Option> { @@ -493,6 +575,10 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { self.err.into_result()?; Ok(self.output) } + + pub fn emit(&mut self, err: impl rustc_errors::Diagnostic<'genv>) { + self.err.collect(self.genv.sess().emit_err(err)); + } } impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { @@ -648,7 +734,7 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { } /// Akin to `rustc_resolve::Module` but specialized to what we support -#[derive(Debug)] +#[derive(Clone, Copy, Debug)] struct Module { kind: ModuleKind, def_id: DefId, @@ -661,7 +747,7 @@ impl Module { } /// Akin to `rustc_resolve::ModuleKind` but specialized to what we support -#[derive(Debug)] +#[derive(Clone, Copy, Debug)] enum ModuleKind { Mod, Trait, @@ -818,11 +904,7 @@ impl<'a, 'genv, 'tcx> ItemResolver<'a, 'genv, 'tcx> { Self { resolver, errors, item_id } } - fn resolve_type_path(&mut self, path: &surface::Path) { - self.resolve_path_in(path, TypeNS); - } - - fn resolve_path_in(&mut self, path: &surface::Path, ns: Namespace) { + fn resolve_path_in(&mut self, ns: Namespace, path: &surface::Path) { if let Some(partial_res) = self.resolver.resolve_path_with_ribs(&path.segments, ns) { self.resolver .output @@ -955,7 +1037,7 @@ impl surface::visit::Visitor for ItemResolver<'_, '_, '_> { }; if !check_ns(TypeNS) && check_ns(ValueNS) { - self.resolve_path_in(path, ValueNS); + self.resolve_path_in(ValueNS, path); return; } } @@ -964,13 +1046,13 @@ impl surface::visit::Visitor for ItemResolver<'_, '_, '_> { fn visit_const_arg(&mut self, const_arg: &surface::ConstArg) { if let surface::ConstArgKind::Path(path) = &const_arg.kind { - self.resolve_path_in(path, ValueNS); + self.resolve_path_in(ValueNS, path); surface::visit::walk_path(self, path); } } fn visit_path(&mut self, path: &surface::Path) { - self.resolve_type_path(path); + self.resolve_path_in(TypeNS, path); surface::visit::walk_path(self, path); } } diff --git a/crates/flux-desugar/src/resolver/refinement_resolver.rs b/crates/flux-desugar/src/resolver/refinement_resolver.rs index a157d83d336..90517df9eb0 100644 --- a/crates/flux-desugar/src/resolver/refinement_resolver.rs +++ b/crates/flux-desugar/src/resolver/refinement_resolver.rs @@ -365,6 +365,10 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { FluxItem::FuncDef(defn) => &defn.sort_vars[..], FluxItem::SortDecl(sort_decl) => &sort_decl.sort_vars[..], FluxItem::Qualifier(_) | FluxItem::PrimOpProp(_) => &[], + FluxItem::Use(_) => { + // Use paths are resolved `CrateResolver::resolve_use_path` + return Ok(()); + } }; Self::new(resolver).run(sort_vars, |r| r.visit_flux_item(item)) } @@ -551,7 +555,7 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { fn emit_unresolved_expr_path(&mut self, path: &surface::ExprPath) { self.errors.emit(super::errors::UnresolvedName { span: path.span, - name: path.segments.iter().map(|s| s.ident).join("::"), + name: path.display(), kind: "value", }); } diff --git a/crates/flux-middle/src/queries.rs b/crates/flux-middle/src/queries.rs index ee0ae57013b..9aa1016160b 100644 --- a/crates/flux-middle/src/queries.rs +++ b/crates/flux-middle/src/queries.rs @@ -400,22 +400,27 @@ impl<'genv, 'tcx> Queries<'genv, 'tcx> { &items .iter() .filter_map(|item| { - let res = match item { - surface::FluxItem::FuncDef(_) => { - fhir::Res::GlobalFunc(fhir::SpecFuncKind::Def( - FluxDefId::new(parent.to_def_id(), item.name().name), - )) + let res; + let ident; + match item { + surface::FluxItem::FuncDef(func) => { + ident = func.name; + res = fhir::Res::GlobalFunc(fhir::SpecFuncKind::Def( + FluxDefId::new(parent.to_def_id(), ident.name), + )); } - surface::FluxItem::SortDecl(_) => { - fhir::Res::UserSort(FluxDefId::new( + surface::FluxItem::SortDecl(sort) => { + ident = sort.name; + res = fhir::Res::UserSort(FluxDefId::new( parent.to_def_id(), - item.name().name, - )) + ident.name, + )); } surface::FluxItem::Qualifier(_) - | surface::FluxItem::PrimOpProp(_) => return None, + | surface::FluxItem::PrimOpProp(_) + | surface::FluxItem::Use(_) => return None, }; - Some(fhir::FluxModChild { ident: item.name(), res }) + Some(fhir::FluxModChild { ident, res }) }) .collect::>(), ) diff --git a/crates/flux-syntax/Cargo.toml b/crates/flux-syntax/Cargo.toml index c32f0fb285a..784283ca3cb 100644 --- a/crates/flux-syntax/Cargo.toml +++ b/crates/flux-syntax/Cargo.toml @@ -11,6 +11,8 @@ test = false [dependencies] flux-macros.workspace = true flux-config.workspace = true + +itertools.workspace = true rustc-hash.workspace = true diff --git a/crates/flux-syntax/src/parser/mod.rs b/crates/flux-syntax/src/parser/mod.rs index a5c6920d88f..588ca74d2ec 100644 --- a/crates/flux-syntax/src/parser/mod.rs +++ b/crates/flux-syntax/src/parser/mod.rs @@ -164,6 +164,7 @@ pub(crate) fn parse_flux_items(cx: &mut ParseCtxt) -> ParseResult> /// | ⟨qualifier⟩ /// | ⟨sort_decl⟩ /// | ⟨primop_prop⟩ +/// | ⟨use_item⟩ /// ``` fn parse_flux_item(cx: &mut ParseCtxt) -> ParseResult { let mut lookahead = cx.lookahead1(); @@ -178,6 +179,8 @@ fn parse_flux_item(cx: &mut ParseCtxt) -> ParseResult { parse_sort_decl(cx).map(FluxItem::SortDecl) } else if lookahead.peek(kw::Property) { parse_primop_property(cx).map(FluxItem::PrimOpProp) + } else if lookahead.peek(kw::Use) { + parse_use_item(cx).map(FluxItem::Use) } else { Err(lookahead.into_error()) } @@ -579,6 +582,16 @@ fn parse_primop_property(cx: &mut ParseCtxt) -> ParseResult { Ok(PrimOpProp { name, op, params, body, span: cx.mk_span(lo, hi) }) } +/// ```text +/// ⟨use_item⟩ := use ⟨expr_path⟩ ; +/// ``` +fn parse_use_item(cx: &mut ParseCtxt) -> ParseResult { + cx.expect(kw::Use)?; + let path = parse_expr_path(cx)?; + cx.expect(token::Semi)?; + Ok(path) +} + pub(crate) fn parse_trait_assoc_refts(cx: &mut ParseCtxt) -> ParseResult> { until(cx, token::Eof, parse_trait_assoc_reft) } diff --git a/crates/flux-syntax/src/surface.rs b/crates/flux-syntax/src/surface.rs index 4ad41ab5da2..7a365bb65a2 100644 --- a/crates/flux-syntax/src/surface.rs +++ b/crates/flux-syntax/src/surface.rs @@ -1,7 +1,9 @@ pub mod visit; + use std::{borrow::Cow, fmt, ops::Range}; use flux_config::PartialInferOpts; +use itertools::Itertools; pub use rustc_ast::{ Mutability, token::{Lit, LitKind}, @@ -35,15 +37,17 @@ pub enum FluxItem { FuncDef(SpecFunc), SortDecl(SortDecl), PrimOpProp(PrimOpProp), + Use(ExprPath), } impl FluxItem { - pub fn name(&self) -> Ident { + pub fn name(&self) -> Option { match self { - FluxItem::Qualifier(qualifier) => qualifier.name, - FluxItem::FuncDef(spec_func) => spec_func.name, - FluxItem::SortDecl(sort_decl) => sort_decl.name, - FluxItem::PrimOpProp(primop_prop) => primop_prop.name, + FluxItem::Qualifier(qualifier) => Some(qualifier.name), + FluxItem::FuncDef(spec_func) => Some(spec_func.name), + FluxItem::SortDecl(sort_decl) => Some(sort_decl.name), + FluxItem::PrimOpProp(primop_prop) => Some(primop_prop.name), + FluxItem::Use(_) => None, } } } @@ -752,6 +756,12 @@ pub struct ExprPath { pub span: Span, } +impl ExprPath { + pub fn display(&self) -> String { + self.segments.iter().map(|s| s.ident).join("::") + } +} + #[derive(Debug, Clone)] pub struct ExprPathSegment { pub ident: Ident, diff --git a/crates/flux-syntax/src/surface/visit.rs b/crates/flux-syntax/src/surface/visit.rs index 4a03efa5c93..8d8190285ba 100644 --- a/crates/flux-syntax/src/surface/visit.rs +++ b/crates/flux-syntax/src/surface/visit.rs @@ -218,6 +218,7 @@ pub fn walk_flux_item(vis: &mut V, item: &FluxItem) { FluxItem::FuncDef(spec_func) => vis.visit_defn(spec_func), FluxItem::SortDecl(sort_decl) => vis.visit_sort_decl(sort_decl), FluxItem::PrimOpProp(prim_op_prop) => vis.visit_primop_prop(prim_op_prop), + FluxItem::Use(qpath) => vis.visit_path_expr(qpath), } } From 214f9f99980a84f6b0d2312112d029ba7df8abda Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 28 Jul 2026 13:22:57 -0400 Subject: [PATCH 04/12] better error messages --- crates/flux-desugar/locales/en-US.ftl | 4 + crates/flux-desugar/src/resolver.rs | 75 ++++++++++++------- .../src/resolver/refinement_resolver.rs | 5 +- crates/flux-syntax/src/surface.rs | 7 -- 4 files changed, 54 insertions(+), 37 deletions(-) diff --git a/crates/flux-desugar/locales/en-US.ftl b/crates/flux-desugar/locales/en-US.ftl index a6a4eac3305..cfdc09d4ce1 100644 --- a/crates/flux-desugar/locales/en-US.ftl +++ b/crates/flux-desugar/locales/en-US.ftl @@ -60,6 +60,10 @@ desugar_unresolved_name = cannot find {$kind} `{$name}` in this scope .label = not found in this scope +desugar_unresolved_import = + unresolved import `{$name}` + .label = {$reason} + desugar_invalid_unrefined_param = invalid use of refinement parameter .label = parameter `{$var}` refers to a type with no indices diff --git a/crates/flux-desugar/src/resolver.rs b/crates/flux-desugar/src/resolver.rs index 1918d089661..6fd9230ee3f 100644 --- a/crates/flux-desugar/src/resolver.rs +++ b/crates/flux-desugar/src/resolver.rs @@ -146,20 +146,11 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { )] fn define_module_flux_items(&mut self, parent: OwnerId) { let Some(items) = self.specs.flux_items_by_parent.get(&parent) else { return }; - // Names are defined per-module so duplicates are only checked within this module. - // let mut definitions = DefinitionMap::default(); for item in items { - // let Some(ident) = item.name() else { continue }; - // // NOTE: This is putting all items in the same namespace. In principle, we could have - // // qualifiers in a different namespace. - // definitions - // .define(ident) - // .emit(&self.genv) - // .collect_err(&mut self.err); - match item { - // Already registered in `define_global_qualifiers_and_primop_props`. - surface::FluxItem::Qualifier(_) | surface::FluxItem::PrimOpProp(_) => {} + surface::FluxItem::Qualifier(_) | surface::FluxItem::PrimOpProp(_) => { + // Already registered in `define_global_qualifiers_and_primop_props`. + } surface::FluxItem::FuncDef(defn) => { let name = defn.name.name; let def_id = FluxDefId::new(parent.def_id.to_def_id(), name); @@ -387,20 +378,29 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { bug!("path must have at least one segment") }; + let not_found_reason = + |ident: Ident, module_id: Option, resolved: &[surface::ExprPathSegment]| { + match module_id { + None => "not found in this scope".to_string(), + Some(_) => format!("no `{ident}` in `{}`", Segment::format_path(resolved)), + } + }; + // 1. Resolve prefix let mut module_id: Option = None; - for segment in prefix { + for (idx, segment) in prefix.iter().enumerate() { + let ident = segment.ident(); let res = if let Some(module_id) = module_id { let module = Module::new(ModuleKind::Mod, module_id); - self.resolve_ident_in_module(module, segment.ident(), TypeNS) + self.resolve_ident_in_module(module, ident, TypeNS) } else { - self.resolve_ident_with_ribs(segment.ident(), TypeNS) + self.resolve_ident_with_ribs(ident, TypeNS) }; let Some(res) = res else { - self.emit(errors::UnresolvedName { - span: segment.ident().span, - name: segment.ident().to_string(), - kind: "import", + self.emit(errors::UnresolvedImport { + span: ident.span, + name: Segment::format_path(&prefix[..=idx]), + reason: not_found_reason(ident, module_id, &prefix[..idx]), }); return vec![]; }; @@ -408,10 +408,10 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { if let Res::Def(DefKind::Mod, def_id) = res { module_id = Some(def_id); } else { - self.emit(errors::UnresolvedName { - span: segment.ident().span, - name: segment.ident().to_string(), - kind: "import", + self.emit(errors::UnresolvedImport { + span: ident.span, + name: Segment::format_path(&prefix[..=idx]), + reason: format!("`{ident}` is not a module"), }); return vec![]; } @@ -433,10 +433,10 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { // 3. Report error if no valid resolution if resolutions.is_empty() { - self.emit(errors::UnresolvedName { + self.emit(errors::UnresolvedImport { span: path.span, - name: path.display(), - kind: "import", + name: Segment::format_path(&path.segments), + reason: not_found_reason(last.ident(), module_id, prefix), }) } @@ -820,6 +820,13 @@ trait Segment: std::fmt::Debug { res: fhir::Res, ); fn ident(&self) -> Ident; + + fn format_path(segments: &[Self]) -> String + where + Self: Sized, + { + segments.iter().map(|s| s.ident()).join("::") + } } impl Segment for surface::PathSegment { @@ -984,7 +991,7 @@ impl<'a, 'genv, 'tcx> ItemResolver<'a, 'genv, 'tcx> { fn emit_unresolved_path(&mut self, path: &surface::Path, ns: Namespace) { self.errors.emit(errors::UnresolvedName { span: path.span, - name: path.segments.iter().map(|segment| segment.ident).join("::"), + name: Segment::format_path(&path.segments), kind: ns.descr(), }); } @@ -1120,6 +1127,20 @@ mod errors { pub name: String, } + /// An import path (`flux::use foo::bar::baz`) that could not be resolved. Unlike + /// [`UnresolvedName`], this always reports the full requested path in the message and + /// explains, via `reason`, what specifically went wrong at the failing segment (not found, + /// or found but not a module). + #[derive(Diagnostic)] + #[diag(desugar_unresolved_import, code = E0999)] + pub(crate) struct UnresolvedImport { + #[primary_span] + #[label] + pub span: Span, + pub name: String, + pub reason: String, + } + #[derive(Diagnostic)] #[diag(desugar_unknown_qualifier, code = E0999)] pub(super) struct UnknownQualifier { diff --git a/crates/flux-desugar/src/resolver/refinement_resolver.rs b/crates/flux-desugar/src/resolver/refinement_resolver.rs index 90517df9eb0..8c35bcb6570 100644 --- a/crates/flux-desugar/src/resolver/refinement_resolver.rs +++ b/crates/flux-desugar/src/resolver/refinement_resolver.rs @@ -14,7 +14,6 @@ use flux_syntax::{ surface::{self, FluxItem, Ident, NodeId, visit::Visitor as _}, walk_list, }; -use itertools::Itertools; use rustc_data_structures::{fx::FxIndexMap, unord::UnordMap}; use rustc_hash::FxHashMap; use rustc_middle::ty::TyCtxt; @@ -555,7 +554,7 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { fn emit_unresolved_expr_path(&mut self, path: &surface::ExprPath) { self.errors.emit(super::errors::UnresolvedName { span: path.span, - name: path.display(), + name: Segment::format_path(&path.segments), kind: "value", }); } @@ -568,7 +567,7 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { .map(|ident| ident.span) .reduce(Span::to) .unwrap_or_default(), - name: path.segments.iter().join("::"), + name: Segment::format_path(&path.segments), kind: "sort", }); } diff --git a/crates/flux-syntax/src/surface.rs b/crates/flux-syntax/src/surface.rs index 7a365bb65a2..2b5a936061d 100644 --- a/crates/flux-syntax/src/surface.rs +++ b/crates/flux-syntax/src/surface.rs @@ -3,7 +3,6 @@ pub mod visit; use std::{borrow::Cow, fmt, ops::Range}; use flux_config::PartialInferOpts; -use itertools::Itertools; pub use rustc_ast::{ Mutability, token::{Lit, LitKind}, @@ -756,12 +755,6 @@ pub struct ExprPath { pub span: Span, } -impl ExprPath { - pub fn display(&self) -> String { - self.segments.iter().map(|s| s.ident).join("::") - } -} - #[derive(Debug, Clone)] pub struct ExprPathSegment { pub ident: Ident, From 830d18b1363e3ea3e029c3a452d05421ce0815f4 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 28 Jul 2026 14:14:24 -0400 Subject: [PATCH 05/12] Add tests --- .../resolver/unresolved_import.rs | 41 ++++++++++++ tests/tests/pos/surface/resolver07.rs | 65 +++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 tests/tests/neg/error_messages/resolver/unresolved_import.rs create mode 100644 tests/tests/pos/surface/resolver07.rs diff --git a/tests/tests/neg/error_messages/resolver/unresolved_import.rs b/tests/tests/neg/error_messages/resolver/unresolved_import.rs new file mode 100644 index 00000000000..01d6a4570d8 --- /dev/null +++ b/tests/tests/neg/error_messages/resolver/unresolved_import.rs @@ -0,0 +1,41 @@ +//! Test the `unresolved import` diagnostic for each failure mode of `resolve_flux_use_path`. +#![allow(dead_code)] + +use flux_attrs::*; + +mod mod_a { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 1 } + + opaque sort Bag; + } + + struct Hidden; +} + +// Single-segment name that doesn't resolve in local scope. +defs! { + use nonexistent; //~ ERROR unresolved import +} + +// First prefix segment doesn't resolve in local scope. +defs! { + use nonexistent::foo; //~ ERROR unresolved import +} + +// `mod_a` resolves, but the item doesn't exist inside it. +defs! { + use mod_a::nonexistent; //~ ERROR unresolved import +} + +// `Bag` resolves inside `mod_a`, but it's a sort, not a module. +defs! { + use mod_a::Bag::x; //~ ERROR unresolved import +} + +// `Hidden` exists in `mod_a` but isn't `pub`. +defs! { + use mod_a::Hidden; //~ ERROR unresolved import +} diff --git a/tests/tests/pos/surface/resolver07.rs b/tests/tests/pos/surface/resolver07.rs new file mode 100644 index 00000000000..7d4d6ba4139 --- /dev/null +++ b/tests/tests/pos/surface/resolver07.rs @@ -0,0 +1,65 @@ +//! Test that `flux::use` imports flux items so they can be referred to unqualified. +#![allow(dead_code)] + +use flux_attrs::*; + +mod mod_a { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 1 } + + opaque sort Bag; + } +} + +// Import a func and a sort from a sibling module. +defs! { + use mod_a::shift; + use mod_a::Bag; +} + +#[sig(fn(x: i32) -> i32[shift(x)])] +pub fn test_use_fn(x: i32) -> i32 { + x + 1 +} + +#[opaque] +#[refined_by(b: Bag)] +pub struct WithSort { + inner: Vec, +} + +mod nested { + pub mod inner { + use flux_attrs::*; + + defs! { + fn dbl(x: int) -> int { 2 * x } + } + } +} + +// Import through a multi-segment (nested module) path. +defs! { + use nested::inner::dbl; +} + +#[sig(fn(x: i32) -> i32[dbl(x)])] +pub fn test_nested_use(x: i32) -> i32 { + 2 * x +} + +mod sibling { + use flux_attrs::*; + + // Import using a `crate::` prefixed path. + defs! { + use crate::nested::inner::dbl; + } + + #[sig(fn(x: i32) -> i32[dbl(x)])] + pub fn test_crate_path_use(x: i32) -> i32 { + 2 * x + } +} From 2a23a89c4c303663d38d63f43ef33bec91646de9 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 28 Jul 2026 14:37:10 -0400 Subject: [PATCH 06/12] Report duplicated items errors --- crates/flux-desugar/locales/en-US.ftl | 4 +- crates/flux-desugar/src/resolver.rs | 153 +++++++++++------- .../src/resolver/refinement_resolver.rs | 32 +--- .../error_messages/desugar/duplicate_param.rs | 2 +- .../error_messages/desugar/index_errors00.rs | 2 +- .../neg/error_messages/desugar/pound_bind.rs | 2 +- .../error_messages/resolver/dup_flux_items.rs | 8 +- 7 files changed, 112 insertions(+), 91 deletions(-) diff --git a/crates/flux-desugar/locales/en-US.ftl b/crates/flux-desugar/locales/en-US.ftl index cfdc09d4ce1..05a84dd3e6d 100644 --- a/crates/flux-desugar/locales/en-US.ftl +++ b/crates/flux-desugar/locales/en-US.ftl @@ -48,8 +48,8 @@ desugar_duplicate_definition = .previous_definition = previous definition of `{$name}` desugar_duplicate_param = - the name `{$name}` is already used as a parameter - .label = already used + identifier `{$name}` is bound more than once in this parameter list + .label = used as a parameter more than once .first_use = first use of `{$name}` desugar_unsupported_signature = diff --git a/crates/flux-desugar/src/resolver.rs b/crates/flux-desugar/src/resolver.rs index 6fd9230ee3f..fbda10fad29 100644 --- a/crates/flux-desugar/src/resolver.rs +++ b/crates/flux-desugar/src/resolver.rs @@ -114,28 +114,46 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } } - /// Qualifiers and primop-props are global + /// Qualifiers and primop-props are global, so their names are checked for duplicates across + /// the whole crate (unlike other flux items, which are scoped per-module via [`Self::define_res_in`]). #[allow( clippy::disallowed_methods, reason = "`flux_items_by_parent` is the source of truth for `FluxDefId`" )] fn define_flux_global_items(&mut self) { + let mut definitions = DefinitionMap::default(); for (parent, items) in &self.specs.flux_items_by_parent { for item in items { + // We are putting qualifiers and primpops in the same namespace. match item { surface::FluxItem::Qualifier(qual) => { - let def_id = FluxLocalDefId::new(parent.def_id, qual.name.name); - self.qualifiers.insert(qual.name.name, def_id); + let ident = qual.name; + if definitions + .define(ident) + .emit(&self.genv) + .collect_err(&mut self.err) + .is_some() + { + let def_id = FluxLocalDefId::new(parent.def_id, ident.name); + self.qualifiers.insert(ident.name, def_id); + } } - surface::FluxItem::PrimOpProp(primop_prop) => { - let def_id = - FluxDefId::new(parent.def_id.to_def_id(), primop_prop.name.name); - self.primop_props.insert(primop_prop.name.name, def_id); + surface::FluxItem::PrimOpProp(primop) => { + let ident = primop.name; + if definitions + .define(ident) + .emit(&self.genv) + .collect_err(&mut self.err) + .is_some() + { + let def_id = FluxDefId::new(parent.def_id.to_def_id(), ident.name); + self.primop_props.insert(ident.name, def_id); + } } surface::FluxItem::Use(_) | surface::FluxItem::FuncDef(_) | surface::FluxItem::SortDecl(_) => {} - } + }; } } } @@ -152,18 +170,17 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { // Already registered in `define_global_qualifiers_and_primop_props`. } surface::FluxItem::FuncDef(defn) => { - let name = defn.name.name; - let def_id = FluxDefId::new(parent.def_id.to_def_id(), name); + let def_id = FluxDefId::new(parent.def_id.to_def_id(), defn.name.name); let kind = fhir::SpecFuncKind::Def(def_id); - self.define_res_in(name, fhir::Res::GlobalFunc(kind), ReftNS); + self.define_res_in(defn.name, fhir::Res::GlobalFunc(kind), ReftNS); } surface::FluxItem::SortDecl(sort_decl) => { let def_id = FluxDefId::new(parent.def_id.to_def_id(), sort_decl.name.name); - self.define_res_in(sort_decl.name.name, fhir::Res::UserSort(def_id), TypeNS); + self.define_res_in(sort_decl.name, fhir::Res::UserSort(def_id), TypeNS); } surface::FluxItem::Use(path) => { for (ident, res, ns) in self.resolve_flux_use_path(path) { - self.define_res_in(ident.name, res, ns); + self.define_res_in(ident, res, ns); } } } @@ -177,16 +194,15 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { ItemKind::Use(path, kind) => { match kind { hir::UseKind::Single(ident) => { - let name = ident.name; if let Some(res) = path.res.value_ns && let Ok(res) = fhir::Res::try_from(res) { - self.define_res_in(name, res, ValueNS); + self.define_res_in(ident, res, ValueNS); } if let Some(res) = path.res.type_ns && let Ok(res) = fhir::Res::try_from(res) { - self.define_res_in(name, res, TypeNS); + self.define_res_in(ident, res, TypeNS); } } hir::UseKind::Glob => { @@ -195,11 +211,10 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { if let Ok(res) = fhir::Res::try_from(mod_child.res) && let Some(ns @ (TypeNS | ValueNS)) = res.ns() { - let name = mod_child.ident.name; if is_prelude { - self.define_in_prelude(name, res, ns); + self.define_in_prelude(mod_child.ident, res, ns); } else { - self.define_res_in(name, res, ns); + self.define_res_in(mod_child.ident, res, ns); } } } @@ -224,11 +239,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { if let Some(ns) = def_kind.ns().map(Namespace::from) && let Some(ident) = item.kind.ident() { - self.define_res_in( - ident.name, - fhir::Res::Def(def_kind, item.owner_id.to_def_id()), - ns, - ); + self.define_res_in(ident, fhir::Res::Def(def_kind, item.owner_id.to_def_id()), ns); } } } @@ -239,7 +250,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { match item.kind { rustc_hir::ForeignItemKind::Type => { self.define_res_in( - item.ident.name, + item.ident, fhir::Res::Def(DefKind::ForeignTy, item.owner_id.to_def_id()), TypeNS, ); @@ -249,25 +260,33 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } } - /// Define `name` in the innermost rib of `ns`. If the rib already binds `name`, keep the existing - /// binding and return it. - fn define_res_in( - &mut self, - name: Symbol, - res: fhir::Res, - ns: Namespace, - ) -> Option> { - match self.ribs[ns].last_mut().unwrap().bindings.entry(name) { - hash_map::Entry::Occupied(entry) => Some(*entry.get()), + /// Define `ident` in the innermost rib of `ns`. If the rib already binds that name, keep the + /// existing binding, report the clash against its original location. + fn define_res_in(&mut self, ident: Ident, res: fhir::Res, ns: Namespace) { + if ident.name == kw::Underscore { + return; + } + match self.ribs[ns].last_mut().unwrap().bindings.entry(ident) { + hash_map::Entry::Occupied(entry) => { + let prev_ident = *entry.key(); + if let fhir::Res::Param(..) = entry.get() { + self.emit(errors::DuplicateParam::new(prev_ident, ident)); + } else { + self.emit(errors::DuplicateDefinition { + span: ident.span, + previous_definition: prev_ident.span, + name: ident, + }); + } + } hash_map::Entry::Vacant(entry) => { entry.insert(res); - None } - } + }; } - fn define_in_prelude(&mut self, name: Symbol, res: fhir::Res, ns: Namespace) { - self.prelude[ns].bindings.insert(name, res); + fn define_in_prelude(&mut self, ident: Ident, res: fhir::Res, ns: Namespace) { + self.prelude[ns].bindings.insert(ident, res); } fn push_rib(&mut self, ns: Namespace, kind: RibKind) { @@ -291,7 +310,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { { debug_assert!(matches!(def_kind, DefKind::TyParam | DefKind::ConstParam)); let param_id = self.genv.maybe_extern_id(param.def_id).resolved_id(); - self.define_res_in(name.name, fhir::Res::Def(def_kind, param_id), ns); + self.define_res_in(name, fhir::Res::Def(def_kind, param_id), ns); } } } @@ -437,7 +456,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { span: path.span, name: Segment::format_path(&path.segments), reason: not_found_reason(last.ident(), module_id, prefix), - }) + }); } resolutions @@ -450,7 +469,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { ) -> Option> { let mut ribs = self.ribs[ns].iter().rev(); while let Some(rib) = ribs.next() { - if let Some(res) = rib.bindings.get(&ident.name) { + if let Some(res) = rib.bindings.get(&ident) { return Some(*res); } match rib.kind { @@ -482,7 +501,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } } - if let Some(res) = self.prelude[ns].bindings.get(&ident.name) { + if let Some(res) = self.prelude[ns].bindings.get(&ident) { return Some(*res); } None @@ -650,7 +669,7 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { ItemKind::Trait(..) => { self.define_generics(def_id); self.define_res_in( - kw::SelfUpper, + Ident::with_dummy_span(kw::SelfUpper), fhir::Res::SelfTyParam { trait_: def_id.resolved_id() }, TypeNS, ); @@ -658,7 +677,7 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { ItemKind::Impl(hir::Impl { of_trait, .. }) => { self.define_generics(def_id); self.define_res_in( - kw::SelfUpper, + Ident::with_dummy_span(kw::SelfUpper), fhir::Res::SelfTyAlias { alias_to: def_id.resolved_id(), is_trait_impl: of_trait.is_some(), @@ -672,7 +691,7 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { ItemKind::Enum(..) => { self.define_generics(def_id); self.define_res_in( - kw::SelfUpper, + Ident::with_dummy_span(kw::SelfUpper), fhir::Res::SelfTyAlias { alias_to: def_id.resolved_id(), is_trait_impl: false }, TypeNS, ); @@ -680,7 +699,7 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { ItemKind::Struct(..) => { self.define_generics(def_id); self.define_res_in( - kw::SelfUpper, + Ident::with_dummy_span(kw::SelfUpper), fhir::Res::SelfTyAlias { alias_to: def_id.resolved_id(), is_trait_impl: false }, TypeNS, ); @@ -775,7 +794,7 @@ pub(crate) enum RibKind { #[derive(Debug)] struct Rib { kind: RibKind, - bindings: UnordMap>, + bindings: UnordMap>, } impl Rib { @@ -1072,10 +1091,10 @@ fn builtin_types_rib() -> Rib { use flux_middle::fhir::PrimSort; let sorts = PrimSort::ALL .into_iter() - .map(|prim| (prim.name(), fhir::Res::PrimSort(prim))); + .map(|prim| (Ident::with_dummy_span(prim.name()), fhir::Res::PrimSort(prim))); let types = PrimTy::ALL .into_iter() - .map(|pty| (pty.name(), fhir::Res::PrimTy(pty))); + .map(|pty| (Ident::with_dummy_span(pty.name()), fhir::Res::PrimTy(pty))); // Types go after such that they override sorts with the same name let bindings = sorts.chain(types).collect(); @@ -1085,13 +1104,15 @@ fn builtin_types_rib() -> Rib { /// The [`Namespace::ReftNS`] prelude: theory functions and `cast`. fn theory_funcs_rib() -> Rib { let mut rib = Rib::new(RibKind::Misc); - rib.bindings.extend_unord( - flux_middle::THEORY_FUNCS - .items() - .map(|(_, itf)| (itf.name, fhir::Res::GlobalFunc(fhir::SpecFuncKind::Thy(itf.itf)))), - ); rib.bindings - .insert(sym::cast, fhir::Res::GlobalFunc(fhir::SpecFuncKind::Cast)); + .extend_unord(flux_middle::THEORY_FUNCS.items().map(|(_, itf)| { + ( + Ident::with_dummy_span(itf.name), + fhir::Res::GlobalFunc(fhir::SpecFuncKind::Thy(itf.itf)), + ) + })); + rib.bindings + .insert(Ident::with_dummy_span(sym::cast), fhir::Res::GlobalFunc(fhir::SpecFuncKind::Cast)); rib } @@ -1111,7 +1132,7 @@ fn mk_crate_mapping(tcx: TyCtxt) -> UnordMap { mod errors { use flux_errors::E0999; use flux_macros::Diagnostic; - use rustc_span::{Ident, Span}; + use rustc_span::{Ident, Span, Symbol}; /// A name that could not be resolved. `kind` is the user-facing description of what was being /// looked for (`"type"`, `"value"`, `"sort"`, ...); it is passed explicitly by each call site @@ -1190,4 +1211,22 @@ mod errors { pub previous_definition: Span, pub name: Ident, } + + #[derive(Diagnostic)] + #[diag(desugar_duplicate_param, code = E0999)] + pub(super) struct DuplicateParam { + #[primary_span] + #[label] + span: Span, + name: Symbol, + #[label(desugar_first_use)] + first_use: Span, + } + + impl DuplicateParam { + pub(super) fn new(old_ident: Ident, new_ident: Ident) -> Self { + debug_assert_eq!(old_ident.name, new_ident.name); + Self { span: new_ident.span, name: new_ident.name, first_use: old_ident.span } + } + } } diff --git a/crates/flux-desugar/src/resolver/refinement_resolver.rs b/crates/flux-desugar/src/resolver/refinement_resolver.rs index 8c35bcb6570..11b54ffd7fb 100644 --- a/crates/flux-desugar/src/resolver/refinement_resolver.rs +++ b/crates/flux-desugar/src/resolver/refinement_resolver.rs @@ -406,7 +406,7 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { self.resolver.push_rib(TypeNS, RibKind::Misc); for (idx, ident) in sort_vars.iter().enumerate() { self.resolver - .define_res_in(ident.name, Res::SortParam(idx), TypeNS); + .define_res_in(*ident, Res::SortParam(idx), TypeNS); } let mut wrapper = self.wrap(); f(&mut wrapper); @@ -425,14 +425,8 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { self.param_defs .insert(param_id, ParamDef { ident, kind, scope }); - if let Some(Res::Param(_, prev_id)) = - self.resolver - .define_res_in(ident.name, Res::Param(kind, param_id), ReftNS) - { - let prev_ident = self.param_defs[&prev_id].ident; - self.errors - .emit(errors::DuplicateParam::new(prev_ident, ident)); - } + self.resolver + .define_res_in(ident, Res::Param(kind, param_id), ReftNS); } fn resolve_path(&mut self, path: &surface::ExprPath) { @@ -741,25 +735,7 @@ mod errors { use flux_errors::E0999; use flux_macros::Diagnostic; use flux_syntax::surface; - use rustc_span::{Span, Symbol, symbol::Ident}; - - #[derive(Diagnostic)] - #[diag(desugar_duplicate_param, code = E0999)] - pub(super) struct DuplicateParam { - #[primary_span] - #[label] - span: Span, - name: Symbol, - #[label(desugar_first_use)] - first_use: Span, - } - - impl DuplicateParam { - pub(super) fn new(old_ident: Ident, new_ident: Ident) -> Self { - debug_assert_eq!(old_ident.name, new_ident.name); - Self { span: new_ident.span, name: new_ident.name, first_use: old_ident.span } - } - } + use rustc_span::{Span, symbol::Ident}; #[derive(Diagnostic)] #[diag(desugar_invalid_unrefined_param, code = E0999)] diff --git a/tests/tests/neg/error_messages/desugar/duplicate_param.rs b/tests/tests/neg/error_messages/desugar/duplicate_param.rs index 646d33aa148..da8928613c7 100644 --- a/tests/tests/neg/error_messages/desugar/duplicate_param.rs +++ b/tests/tests/neg/error_messages/desugar/duplicate_param.rs @@ -10,5 +10,5 @@ - xanadu:i32))] //~ ERROR the name `xanadu` is already used as a parameter + xanadu:i32))] //~ ERROR identifier `xanadu` is bound more than once pub fn test00(_x: i32, _y: i32) {} diff --git a/tests/tests/neg/error_messages/desugar/index_errors00.rs b/tests/tests/neg/error_messages/desugar/index_errors00.rs index 622d8bb029d..a6825d3dda8 100644 --- a/tests/tests/neg/error_messages/desugar/index_errors00.rs +++ b/tests/tests/neg/error_messages/desugar/index_errors00.rs @@ -16,7 +16,7 @@ fn dipa(f: &mut f32) -> i32 { 0 } -#[flux::sig(fn(x: i32, i32[@x]))] //~ ERROR the name `x` is already used +#[flux::sig(fn(x: i32, i32[@x]))] //~ ERROR identifier `x` is bound more than once fn stout(x: i32, y: i32) {} #[flux::refined_by()] diff --git a/tests/tests/neg/error_messages/desugar/pound_bind.rs b/tests/tests/neg/error_messages/desugar/pound_bind.rs index 24120ddfb33..1f3b0637f44 100644 --- a/tests/tests/neg/error_messages/desugar/pound_bind.rs +++ b/tests/tests/neg/error_messages/desugar/pound_bind.rs @@ -1,4 +1,4 @@ -#[flux::sig(fn() -> (i32[#n], i32[#n]))] //~ ERROR the name `n` is already used as a parameter +#[flux::sig(fn() -> (i32[#n], i32[#n]))] //~ ERROR identifier `n` is bound more than once fn test00() -> (i32, i32) { (0, 0) } diff --git a/tests/tests/neg/error_messages/resolver/dup_flux_items.rs b/tests/tests/neg/error_messages/resolver/dup_flux_items.rs index cba170daf06..5c50d4f918d 100644 --- a/tests/tests/neg/error_messages/resolver/dup_flux_items.rs +++ b/tests/tests/neg/error_messages/resolver/dup_flux_items.rs @@ -5,7 +5,13 @@ defs! { fn foo() -> int; //~ ERROR name `foo` is defined multiple times - qualifier foo(x: int) { //~ ERROR name `foo` is defined multiple times + // Qualifiers are checked in a separate, crate-global namespace, so this doesn't clash + // with `foo` above. + qualifier foo(x: int) { + x > 0 + } + + qualifier foo(x: int) { //~ ERROR name `foo` is defined multiple times x > 0 } } From b9b6d36bbe83444933300584c0f40aba983d9625 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 28 Jul 2026 15:07:44 -0400 Subject: [PATCH 07/12] add tests --- .../error_messages/resolver/dup_flux_items.rs | 26 ++++++++++++++ .../resolver/duplicate_import.rs | 36 +++++++++++++++++++ tests/tests/pos/surface/resolver08.rs | 11 ++++++ 3 files changed, 73 insertions(+) create mode 100644 tests/tests/neg/error_messages/resolver/duplicate_import.rs create mode 100644 tests/tests/pos/surface/resolver08.rs diff --git a/tests/tests/neg/error_messages/resolver/dup_flux_items.rs b/tests/tests/neg/error_messages/resolver/dup_flux_items.rs index 5c50d4f918d..173d5dd1c40 100644 --- a/tests/tests/neg/error_messages/resolver/dup_flux_items.rs +++ b/tests/tests/neg/error_messages/resolver/dup_flux_items.rs @@ -5,6 +5,10 @@ defs! { fn foo() -> int; //~ ERROR name `foo` is defined multiple times + opaque sort Bag; + + opaque sort Bag; //~ ERROR name `Bag` is defined multiple times + // Qualifiers are checked in a separate, crate-global namespace, so this doesn't clash // with `foo` above. qualifier foo(x: int) { @@ -15,3 +19,25 @@ defs! { x > 0 } } + +// Qualifiers (and primop-props) are global: `bar` clashes across module boundaries too, unlike +// funcs/sorts which are scoped per-module. +mod mod_a { + use flux_attrs::*; + + defs! { + qualifier bar(x: int) { + x > 0 + } + } +} + +mod mod_b { + use flux_attrs::*; + + defs! { + qualifier bar(x: int) { //~ ERROR name `bar` is defined multiple times + x > 0 + } + } +} diff --git a/tests/tests/neg/error_messages/resolver/duplicate_import.rs b/tests/tests/neg/error_messages/resolver/duplicate_import.rs new file mode 100644 index 00000000000..5bb03690a49 --- /dev/null +++ b/tests/tests/neg/error_messages/resolver/duplicate_import.rs @@ -0,0 +1,36 @@ +//! Test that `use` participates in duplicate-definition checking like any other flux item, +//! matching rustc's E0252 (use-vs-use) and E0255 (use-vs-item). +#![allow(dead_code)] + +use flux_attrs::*; + +mod mod_a { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 1 } + fn dbl(x: int) -> int { 2 * x } + } +} + +mod mod_b { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 2 } + } +} + +// `use` vs. an item already defined in the importing module. +defs! { + fn dbl(x: int) -> int { 2 * x } + + use mod_a::dbl; //~ ERROR name `dbl` is defined multiple times +} + +// `use` vs. another `use` importing the same name from a different path. +defs! { + use mod_a::shift; + + use mod_b::shift; //~ ERROR name `shift` is defined multiple times +} diff --git a/tests/tests/pos/surface/resolver08.rs b/tests/tests/pos/surface/resolver08.rs new file mode 100644 index 00000000000..432ffc2dc68 --- /dev/null +++ b/tests/tests/pos/surface/resolver08.rs @@ -0,0 +1,11 @@ +//! Test that `_` is never treated as a duplicate definition (matching rustc). +#![allow(dead_code)] + +const _: i32 = 0; +const _: i32 = 0; +const _: i32 = 0; + +mod nested { + const _: i32 = 0; + const _: i32 = 0; +} From c2c0a31c59402d7fe634a9b35fa60c3a16fe0f95 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 28 Jul 2026 15:39:37 -0400 Subject: [PATCH 08/12] support for nested imports --- crates/flux-desugar/src/resolver.rs | 142 +++++++++++------- .../src/resolver/refinement_resolver.rs | 4 +- crates/flux-syntax/src/parser/mod.rs | 25 ++- crates/flux-syntax/src/surface.rs | 16 +- crates/flux-syntax/src/surface/visit.rs | 20 ++- 5 files changed, 137 insertions(+), 70 deletions(-) diff --git a/crates/flux-desugar/src/resolver.rs b/crates/flux-desugar/src/resolver.rs index fbda10fad29..c23de9ad833 100644 --- a/crates/flux-desugar/src/resolver.rs +++ b/crates/flux-desugar/src/resolver.rs @@ -178,8 +178,8 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { let def_id = FluxDefId::new(parent.def_id.to_def_id(), sort_decl.name.name); self.define_res_in(sort_decl.name, fhir::Res::UserSort(def_id), TypeNS); } - surface::FluxItem::Use(path) => { - for (ident, res, ns) in self.resolve_flux_use_path(path) { + surface::FluxItem::Use(use_tree) => { + for (ident, res, ns) in self.resolve_flux_use_tree(use_tree) { self.define_res_in(ident, res, ns); } } @@ -388,78 +388,81 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { None } - fn resolve_flux_use_path( + fn resolve_flux_use_tree( &mut self, - path: &surface::ExprPath, + use_tree: &surface::UseTree, ) -> Vec<(Ident, fhir::Res, Namespace)> { - use fhir::Res; - let [prefix @ .., last] = &path.segments[..] else { + self.resolve_flux_use_tree_rec(use_tree, None, &mut vec![]) + } + + fn resolve_flux_use_tree_rec( + &mut self, + use_tree: &surface::UseTree, + mut resolved_module_id: Option, + resolved_prefix: &mut Vec, + ) -> Vec<(Ident, fhir::Res, Namespace)> { + let Some((last, all_but_last)) = use_tree.prefix.segments.split_last() else { bug!("path must have at least one segment") }; + let module_segments = match &use_tree.kind { + surface::UseTreeKind::Simple => all_but_last, + surface::UseTreeKind::Nested(_) => &use_tree.prefix.segments[..], + }; - let not_found_reason = - |ident: Ident, module_id: Option, resolved: &[surface::ExprPathSegment]| { - match module_id { - None => "not found in this scope".to_string(), - Some(_) => format!("no `{ident}` in `{}`", Segment::format_path(resolved)), - } - }; - - // 1. Resolve prefix - let mut module_id: Option = None; - for (idx, segment) in prefix.iter().enumerate() { + for segment in module_segments { let ident = segment.ident(); - let res = if let Some(module_id) = module_id { + let res = if let Some(module_id) = resolved_module_id { let module = Module::new(ModuleKind::Mod, module_id); self.resolve_ident_in_module(module, ident, TypeNS) } else { self.resolve_ident_with_ribs(ident, TypeNS) }; let Some(res) = res else { - self.emit(errors::UnresolvedImport { - span: ident.span, - name: Segment::format_path(&prefix[..=idx]), - reason: not_found_reason(ident, module_id, &prefix[..idx]), - }); + self.emit_unresolved_import(ident, resolved_module_id, resolved_prefix); return vec![]; }; - - if let Res::Def(DefKind::Mod, def_id) = res { - module_id = Some(def_id); + if let fhir::Res::Def(DefKind::Mod, def_id) = res { + resolved_module_id = Some(def_id); } else { - self.emit(errors::UnresolvedImport { - span: ident.span, - name: Segment::format_path(&prefix[..=idx]), - reason: format!("`{ident}` is not a module"), - }); + self.emit_not_a_module(ident, resolved_prefix); return vec![]; } + resolved_prefix.push(ident); } - // 2. Resolve last ident in all namespaces - let mut resolutions = vec![]; - for ns in [TypeNS, ValueNS, ReftNS] { - let res = if let Some(module_id) = module_id { - let module = Module::new(ModuleKind::Mod, module_id); - self.resolve_ident_in_module(module, last.ident(), ns) - } else { - self.resolve_ident_with_ribs(last.ident(), ns) - }; - if let Some(res) = res { - resolutions.push((last.ident(), res, ns)); + match &use_tree.kind { + surface::UseTreeKind::Simple => { + let mut resolutions = vec![]; + for ns in [TypeNS, ValueNS, ReftNS] { + let res = if let Some(module_id) = resolved_module_id { + let module = Module::new(ModuleKind::Mod, module_id); + self.resolve_ident_in_module(module, last.ident(), ns) + } else { + self.resolve_ident_with_ribs(last.ident(), ns) + }; + if let Some(res) = res { + resolutions.push((last.ident(), res, ns)); + } + } + if resolutions.is_empty() { + self.emit_unresolved_import(last.ident, resolved_module_id, resolved_prefix); + } + resolutions + } + surface::UseTreeKind::Nested(items) => { + let mut resolutions = vec![]; + for item in items { + let len = resolved_prefix.len(); + resolutions.extend(self.resolve_flux_use_tree_rec( + item, + resolved_module_id, + resolved_prefix, + )); + resolved_prefix.truncate(len); + } + resolutions } } - - // 3. Report error if no valid resolution - if resolutions.is_empty() { - self.emit(errors::UnresolvedImport { - span: path.span, - name: Segment::format_path(&path.segments), - reason: not_found_reason(last.ident(), module_id, prefix), - }); - } - - resolutions } fn resolve_ident_with_ribs( @@ -595,7 +598,30 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { Ok(self.output) } - pub fn emit(&mut self, err: impl rustc_errors::Diagnostic<'genv>) { + fn emit_unresolved_import( + &mut self, + ident: Ident, + module_id: Option, + resolved_prefix: &[Ident], + ) { + let reason = match module_id { + None => "not found in this scope".to_string(), + Some(_) => format!("no `{ident}` in `{}`", Segment::format_iter(resolved_prefix)), + }; + let name = Segment::format_iter(resolved_prefix.iter().chain(&[ident])); + self.emit(errors::UnresolvedImport { span: ident.span, name, reason }); + } + + fn emit_not_a_module(&mut self, ident: Ident, resolved_prefix: &[Ident]) { + let name = Segment::format_iter(resolved_prefix.iter().chain(&[ident])); + self.emit(errors::UnresolvedImport { + span: ident.span, + name, + reason: format!("`{ident}` is not a module"), + }); + } + + fn emit(&mut self, err: impl rustc_errors::Diagnostic<'genv>) { self.err.collect(self.genv.sess().emit_err(err)); } } @@ -840,11 +866,11 @@ trait Segment: std::fmt::Debug { ); fn ident(&self) -> Ident; - fn format_path(segments: &[Self]) -> String + fn format_iter<'a>(segments: impl IntoIterator) -> String where - Self: Sized, + Self: Sized + 'a, { - segments.iter().map(|s| s.ident()).join("::") + segments.into_iter().map(|s| s.ident()).join("::") } } @@ -1010,7 +1036,7 @@ impl<'a, 'genv, 'tcx> ItemResolver<'a, 'genv, 'tcx> { fn emit_unresolved_path(&mut self, path: &surface::Path, ns: Namespace) { self.errors.emit(errors::UnresolvedName { span: path.span, - name: Segment::format_path(&path.segments), + name: Segment::format_iter(&path.segments), kind: ns.descr(), }); } diff --git a/crates/flux-desugar/src/resolver/refinement_resolver.rs b/crates/flux-desugar/src/resolver/refinement_resolver.rs index 11b54ffd7fb..6eb03a7b5b4 100644 --- a/crates/flux-desugar/src/resolver/refinement_resolver.rs +++ b/crates/flux-desugar/src/resolver/refinement_resolver.rs @@ -548,7 +548,7 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { fn emit_unresolved_expr_path(&mut self, path: &surface::ExprPath) { self.errors.emit(super::errors::UnresolvedName { span: path.span, - name: Segment::format_path(&path.segments), + name: Segment::format_iter(&path.segments), kind: "value", }); } @@ -561,7 +561,7 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { .map(|ident| ident.span) .reduce(Span::to) .unwrap_or_default(), - name: Segment::format_path(&path.segments), + name: Segment::format_iter(&path.segments), kind: "sort", }); } diff --git a/crates/flux-syntax/src/parser/mod.rs b/crates/flux-syntax/src/parser/mod.rs index 588ca74d2ec..c25dc597546 100644 --- a/crates/flux-syntax/src/parser/mod.rs +++ b/crates/flux-syntax/src/parser/mod.rs @@ -24,7 +24,7 @@ use crate::{ ParamMode, Path, PathSegment, PrimOpProp, Qualifier, QualifierKind, QuantKind, RefineArg, RefineParam, RefineParams, Requires, Sort, SortDecl, SortPath, SpecFunc, Spread, StaticInfo, StructDef, TraitAssocReft, TraitRef, Trusted, Ty, TyAlias, TyKind, UnOp, - VariantDef, VariantRet, WhereBoundPredicate, + UseTree, UseTreeKind, VariantDef, VariantRet, WhereBoundPredicate, }, symbols::{kw, sym}, token::{self, Comma, Delimiter::*, IdentIsRaw, Or, Token, TokenKind}, @@ -583,13 +583,28 @@ fn parse_primop_property(cx: &mut ParseCtxt) -> ParseResult { } /// ```text -/// ⟨use_item⟩ := use ⟨expr_path⟩ ; +/// ⟨use_item⟩ := use ⟨use_tree⟩ ; /// ``` -fn parse_use_item(cx: &mut ParseCtxt) -> ParseResult { +fn parse_use_item(cx: &mut ParseCtxt) -> ParseResult { cx.expect(kw::Use)?; - let path = parse_expr_path(cx)?; + let tree = parse_use_tree(cx)?; cx.expect(token::Semi)?; - Ok(path) + Ok(tree) +} + +/// ```text +/// ⟨use_tree⟩ := ⟨expr_path⟩ +/// | ⟨expr_path⟩ { ⟨use_tree⟩,* } +/// ``` +fn parse_use_tree(cx: &mut ParseCtxt) -> ParseResult { + let prefix = parse_expr_path(cx)?; + let kind = if cx.advance_if(token::OpenBrace) { + let items = punctuated_until(cx, token::Comma, token::CloseBrace, parse_use_tree)?; + UseTreeKind::Nested(items) + } else { + UseTreeKind::Simple + }; + Ok(UseTree { prefix, kind }) } pub(crate) fn parse_trait_assoc_refts(cx: &mut ParseCtxt) -> ParseResult> { diff --git a/crates/flux-syntax/src/surface.rs b/crates/flux-syntax/src/surface.rs index 2b5a936061d..14ac0f56b43 100644 --- a/crates/flux-syntax/src/surface.rs +++ b/crates/flux-syntax/src/surface.rs @@ -36,7 +36,7 @@ pub enum FluxItem { FuncDef(SpecFunc), SortDecl(SortDecl), PrimOpProp(PrimOpProp), - Use(ExprPath), + Use(UseTree), } impl FluxItem { @@ -51,6 +51,20 @@ impl FluxItem { } } +#[derive(Debug)] +pub struct UseTree { + pub prefix: ExprPath, + pub kind: UseTreeKind, +} + +#[derive(Debug)] +pub enum UseTreeKind { + /// `use a::b::c` + Simple, + /// `use a::b::{...}` + Nested(Vec), +} + #[derive(Debug)] pub struct Qualifier { pub name: Ident, diff --git a/crates/flux-syntax/src/surface/visit.rs b/crates/flux-syntax/src/surface/visit.rs index 8d8190285ba..32506074897 100644 --- a/crates/flux-syntax/src/surface/visit.rs +++ b/crates/flux-syntax/src/surface/visit.rs @@ -12,10 +12,10 @@ use super::{ Ensures, EnumDef, Expr, ExprKind, ExprPath, ExprPathSegment, FieldExpr, FnInput, FnOutput, FnRetTy, FnSig, GenericArg, GenericArgKind, GenericParam, Generics, Impl, ImplAssocReft, Indices, ItemKind, Lit, Path, PathSegment, Qualifier, RefineArg, RefineParam, Sort, SortPath, - SpecFunc, StructDef, Trait, TraitAssocReft, TraitRef, Ty, TyAlias, TyKind, VariantDef, - VariantRet, WhereBoundPredicate, + SpecFunc, StructDef, Trait, TraitAssocReft, TraitRef, Ty, TyAlias, TyKind, UseTreeKind, + VariantDef, VariantRet, WhereBoundPredicate, }; -use crate::surface::{FluxItem, ImplItemFn, Item, PrimOpProp, SortDecl, TraitItemFn}; +use crate::surface::{FluxItem, ImplItemFn, Item, PrimOpProp, SortDecl, TraitItemFn, UseTree}; #[macro_export] macro_rules! walk_list { @@ -218,7 +218,19 @@ pub fn walk_flux_item(vis: &mut V, item: &FluxItem) { FluxItem::FuncDef(spec_func) => vis.visit_defn(spec_func), FluxItem::SortDecl(sort_decl) => vis.visit_sort_decl(sort_decl), FluxItem::PrimOpProp(prim_op_prop) => vis.visit_primop_prop(prim_op_prop), - FluxItem::Use(qpath) => vis.visit_path_expr(qpath), + FluxItem::Use(use_tree) => walk_use_tree(vis, use_tree), + } +} + +pub fn walk_use_tree(vis: &mut V, use_tree: &UseTree) { + vis.visit_path_expr(&use_tree.prefix); + match &use_tree.kind { + UseTreeKind::Simple => {} + UseTreeKind::Nested(items) => { + for item in items { + walk_use_tree(vis, item); + } + } } } From 3a40743d0a22504ad604e24219c98b0d1768f260 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 28 Jul 2026 17:13:23 -0400 Subject: [PATCH 09/12] Add tests for nested imports --- crates/flux-syntax/src/parser/mod.rs | 24 ++-- .../error_messages/resolver/nested_import.rs | 50 ++++++++ tests/tests/pos/surface/resolver09.rs | 83 +++++++++++++ tests/tests/pos/surface/resolver10.rs | 110 ++++++++++++++++++ 4 files changed, 259 insertions(+), 8 deletions(-) create mode 100644 tests/tests/neg/error_messages/resolver/nested_import.rs create mode 100644 tests/tests/pos/surface/resolver09.rs create mode 100644 tests/tests/pos/surface/resolver10.rs diff --git a/crates/flux-syntax/src/parser/mod.rs b/crates/flux-syntax/src/parser/mod.rs index c25dc597546..8a508d366d3 100644 --- a/crates/flux-syntax/src/parser/mod.rs +++ b/crates/flux-syntax/src/parser/mod.rs @@ -593,17 +593,25 @@ fn parse_use_item(cx: &mut ParseCtxt) -> ParseResult { } /// ```text -/// ⟨use_tree⟩ := ⟨expr_path⟩ -/// | ⟨expr_path⟩ { ⟨use_tree⟩,* } +/// ⟨use_tree⟩ := ⟨ident⟩ ( :: ⟨ident⟩ )* ( :: { ⟨use_tree⟩,* } )? /// ``` fn parse_use_tree(cx: &mut ParseCtxt) -> ParseResult { - let prefix = parse_expr_path(cx)?; - let kind = if cx.advance_if(token::OpenBrace) { - let items = punctuated_until(cx, token::Comma, token::CloseBrace, parse_use_tree)?; - UseTreeKind::Nested(items) - } else { - UseTreeKind::Simple + let lo = cx.lo(); + let mut segments = vec![parse_expr_path_segment(cx)?]; + let mut hi = cx.hi(); + let kind = loop { + if !cx.advance_if(token::PathSep) { + break UseTreeKind::Simple; + } + if cx.advance_if(token::OpenBrace) { + let items = punctuated_until(cx, token::Comma, token::CloseBrace, parse_use_tree)?; + cx.expect(token::CloseBrace)?; + break UseTreeKind::Nested(items); + } + segments.push(parse_expr_path_segment(cx)?); + hi = cx.hi(); }; + let prefix = ExprPath { segments, node_id: cx.next_node_id(), span: cx.mk_span(lo, hi) }; Ok(UseTree { prefix, kind }) } diff --git a/tests/tests/neg/error_messages/resolver/nested_import.rs b/tests/tests/neg/error_messages/resolver/nested_import.rs new file mode 100644 index 00000000000..5fc82298f2c --- /dev/null +++ b/tests/tests/neg/error_messages/resolver/nested_import.rs @@ -0,0 +1,50 @@ +//! Test error reporting for nested imports (`use a::{b, c}`). +#![allow(dead_code)] + +use flux_attrs::*; + +mod mod_a { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 1 } + fn dbl(x: int) -> int { 2 * x } + opaque sort Bag; + } + + pub mod mod_b { + use flux_attrs::*; + + defs! { + fn triple(x: int) -> int { 3 * x } + } + } +} + +// A failing sibling must still be reported even though an earlier sibling in the same list +// resolved successfully. +defs! { + use mod_a::{shift, nonexistent}; //~ ERROR unresolved import +} + +// `Bag` resolves (it's a sort), but it's used here as if it were a module. +defs! { + use mod_a::{Bag::x}; //~ ERROR unresolved import +} + +// Duplicate import of the same name within one nested list. +defs! { + use mod_a::{dbl, dbl}; //~ ERROR name `dbl` is defined multiple times +} + +// A failing branch nested two levels deep must still be reported even though a sibling deep +// branch (also two levels deep) resolves successfully. +defs! { + use mod_a::{mod_b::{nonexistent}, mod_b::{triple}}; //~ ERROR unresolved import +} + +// `triple` resolves (it's a func), but it's used here as if it were a module, discovered from +// inside a nested group rather than at the top level. +defs! { + use mod_a::{mod_b::{triple::x}}; //~ ERROR unresolved import +} diff --git a/tests/tests/pos/surface/resolver09.rs b/tests/tests/pos/surface/resolver09.rs new file mode 100644 index 00000000000..51e38e3cf1e --- /dev/null +++ b/tests/tests/pos/surface/resolver09.rs @@ -0,0 +1,83 @@ +//! Test that nested imports (`use a::{b, c}`) resolve correctly. +#![allow(dead_code)] + +use flux_attrs::*; + +mod mod_a { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 1 } + fn dbl(x: int) -> int { 2 * x } + } + + pub mod mod_b { + use flux_attrs::*; + + defs! { + fn c(x: int) -> int { x + 1 } + fn d(x: int) -> int { x + 2 } + } + } + + pub mod nested { + use flux_attrs::*; + + defs! { + fn dbl(x: int) -> int { 3 * x } + } + } +} + +// Basic nested import: two items from the same module. +defs! { + use mod_a::{shift, dbl}; +} + +#[sig(fn(x: i32) -> i32[shift(x)])] +pub fn test_basic_nested(x: i32) -> i32 { + x + 1 +} + +#[sig(fn(x: i32) -> i32[dbl(x)])] +pub fn test_basic_nested2(x: i32) -> i32 { + 2 * x +} + +mod use_multi_segment { + use flux_attrs::*; + + // Multi-segment prefix before the braces: both `mod_a` and `mod_b` must resolve as modules. + defs! { + use crate::mod_a::mod_b::{c, d}; + } + + #[sig(fn(x: i32) -> i32[c(x)])] + pub fn test_multi_segment_prefix(x: i32) -> i32 { + x + 1 + } + + #[sig(fn(x: i32) -> i32[d(x)])] + pub fn test_multi_segment_prefix2(x: i32) -> i32 { + x + 2 + } +} + +mod use_mixed { + use flux_attrs::*; + + // Mix a plain item with a further-nested path inside the same braces. + defs! { + use crate::mod_a::{shift, nested::dbl}; + } + + #[sig(fn(x: i32) -> i32[shift(x)])] + pub fn test_mixed(x: i32) -> i32 { + x + 1 + } + + #[sig(fn(x: i32) -> i32[dbl(x)])] + pub fn test_mixed2(x: i32) -> i32 { + 3 * x + } +} diff --git a/tests/tests/pos/surface/resolver10.rs b/tests/tests/pos/surface/resolver10.rs new file mode 100644 index 00000000000..4beb6f461a3 --- /dev/null +++ b/tests/tests/pos/surface/resolver10.rs @@ -0,0 +1,110 @@ +//! Test deeper/multi-level nested imports. +#![allow(dead_code)] + +use flux_attrs::*; + +mod mod_a { + use flux_attrs::*; + + defs! { + fn shift(x: int) -> int { x + 1 } + fn dbl(x: int) -> int { 2 * x } + } + + pub mod mod_b { + use flux_attrs::*; + + defs! { + fn triple(x: int) -> int { 3 * x } + fn quad(x: int) -> int { 4 * x } + } + + pub mod mod_x { + use flux_attrs::*; + + defs! { + fn leaf(x: int) -> int { x + 3 } + } + } + } + + pub mod mod_c { + use flux_attrs::*; + + defs! { + fn plus5(x: int) -> int { x + 5 } + } + } +} + +// Nested-within-nested: a nested group containing another nested group alongside a plain item. +defs! { + use mod_a::{mod_b::{triple, quad}, shift}; +} + +#[sig(fn(x: i32) -> i32[triple(x)])] +pub fn test_nested_in_nested_triple(x: i32) -> i32 { + 3 * x +} + +#[sig(fn(x: i32) -> i32[quad(x)])] +pub fn test_nested_in_nested_quad(x: i32) -> i32 { + 4 * x +} + +#[sig(fn(x: i32) -> i32[shift(x)])] +pub fn test_nested_in_nested_shift(x: i32) -> i32 { + x + 1 +} + +mod three_levels { + use flux_attrs::*; + + // Three levels deep. + defs! { + use crate::mod_a::{mod_b::{mod_x::{leaf}}}; + } + + #[sig(fn(x: i32) -> i32[leaf(x)])] + pub fn test_three_levels(x: i32) -> i32 { + x + 3 + } +} + +mod independent_siblings { + use flux_attrs::*; + + // Independent sibling groups: state from the `mod_b` branch must not leak into `mod_c`. + defs! { + use crate::mod_a::{mod_b::{triple}, mod_c::{plus5}}; + } + + #[sig(fn(x: i32) -> i32[triple(x)])] + pub fn test_sibling_triple(x: i32) -> i32 { + 3 * x + } + + #[sig(fn(x: i32) -> i32[plus5(x)])] + pub fn test_sibling_plus5(x: i32) -> i32 { + x + 5 + } +} + +mod trailing_comma { + use flux_attrs::*; + + // Trailing comma inside a nested group. + defs! { + use crate::mod_a::{shift, dbl,}; + } + + #[sig(fn(x: i32) -> i32[shift(x)])] + pub fn test_trailing_comma_shift(x: i32) -> i32 { + x + 1 + } + + #[sig(fn(x: i32) -> i32[dbl(x)])] + pub fn test_trailing_comma_dbl(x: i32) -> i32 { + 2 * x + } +} From 7fc6c19001e6c40865ea6abf6fdc91a197761351 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 28 Jul 2026 19:31:37 -0400 Subject: [PATCH 10/12] Fix resolution inside macros --- crates/flux-desugar/src/resolver.rs | 63 +++++++++++++++++---------- tests/tests/pos/surface/resolver11.rs | 52 ++++++++++++++++++++++ 2 files changed, 91 insertions(+), 24 deletions(-) create mode 100644 tests/tests/pos/surface/resolver11.rs diff --git a/crates/flux-desugar/src/resolver.rs b/crates/flux-desugar/src/resolver.rs index c23de9ad833..1e1743e62e9 100644 --- a/crates/flux-desugar/src/resolver.rs +++ b/crates/flux-desugar/src/resolver.rs @@ -266,27 +266,29 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { if ident.name == kw::Underscore { return; } - match self.ribs[ns].last_mut().unwrap().bindings.entry(ident) { + match self.ribs[ns].last_mut().unwrap().bindings.entry(ident.name) { hash_map::Entry::Occupied(entry) => { - let prev_ident = *entry.key(); - if let fhir::Res::Param(..) = entry.get() { - self.emit(errors::DuplicateParam::new(prev_ident, ident)); + let prev = *entry.get(); + if let fhir::Res::Param(..) = prev.res { + self.emit(errors::DuplicateParam::new(prev.ident, ident)); } else { self.emit(errors::DuplicateDefinition { span: ident.span, - previous_definition: prev_ident.span, + previous_definition: prev.ident.span, name: ident, }); } } hash_map::Entry::Vacant(entry) => { - entry.insert(res); + entry.insert(Binding { ident, res }); } }; } fn define_in_prelude(&mut self, ident: Ident, res: fhir::Res, ns: Namespace) { - self.prelude[ns].bindings.insert(ident, res); + self.prelude[ns] + .bindings + .insert(ident.name, Binding { ident, res }); } fn push_rib(&mut self, ns: Namespace, kind: RibKind) { @@ -472,8 +474,8 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { ) -> Option> { let mut ribs = self.ribs[ns].iter().rev(); while let Some(rib) = ribs.next() { - if let Some(res) = rib.bindings.get(&ident) { - return Some(*res); + if let Some(binding) = rib.bindings.get(&ident.name) { + return Some(binding.res); } match rib.kind { // A module boundary stops item resolution. @@ -504,8 +506,8 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } } - if let Some(res) = self.prelude[ns].bindings.get(&ident) { - return Some(*res); + if let Some(binding) = self.prelude[ns].bindings.get(&ident.name) { + return Some(binding.res); } None } @@ -817,10 +819,19 @@ pub(crate) enum RibKind { FnTraitInput, } +/// The value stored for each binding in a [`Rib`]. `ident` is the original identifier (kept for +/// its span, used to point at the previous location on a duplicate-definition clash); lookups +/// and clash detection are keyed by `ident.name` alone. +#[derive(Clone, Copy, Debug)] +struct Binding { + ident: Ident, + res: fhir::Res, +} + #[derive(Debug)] struct Rib { kind: RibKind, - bindings: UnordMap>, + bindings: UnordMap, } impl Rib { @@ -1115,12 +1126,14 @@ impl surface::visit::Visitor for ItemResolver<'_, '_, '_> { /// derived in `conv_sort_path` (see [`fhir::PrimSort`]). fn builtin_types_rib() -> Rib { use flux_middle::fhir::PrimSort; - let sorts = PrimSort::ALL - .into_iter() - .map(|prim| (Ident::with_dummy_span(prim.name()), fhir::Res::PrimSort(prim))); - let types = PrimTy::ALL - .into_iter() - .map(|pty| (Ident::with_dummy_span(pty.name()), fhir::Res::PrimTy(pty))); + let sorts = PrimSort::ALL.into_iter().map(|prim| { + let ident = Ident::with_dummy_span(prim.name()); + (ident.name, Binding { ident, res: fhir::Res::PrimSort(prim) }) + }); + let types = PrimTy::ALL.into_iter().map(|pty| { + let ident = Ident::with_dummy_span(pty.name()); + (ident.name, Binding { ident, res: fhir::Res::PrimTy(pty) }) + }); // Types go after such that they override sorts with the same name let bindings = sorts.chain(types).collect(); @@ -1132,13 +1145,15 @@ fn theory_funcs_rib() -> Rib { let mut rib = Rib::new(RibKind::Misc); rib.bindings .extend_unord(flux_middle::THEORY_FUNCS.items().map(|(_, itf)| { - ( - Ident::with_dummy_span(itf.name), - fhir::Res::GlobalFunc(fhir::SpecFuncKind::Thy(itf.itf)), - ) + let ident = Ident::with_dummy_span(itf.name); + let res = fhir::Res::GlobalFunc(fhir::SpecFuncKind::Thy(itf.itf)); + (ident.name, Binding { ident, res }) })); - rib.bindings - .insert(Ident::with_dummy_span(sym::cast), fhir::Res::GlobalFunc(fhir::SpecFuncKind::Cast)); + let cast_ident = Ident::with_dummy_span(sym::cast); + rib.bindings.insert( + cast_ident.name, + Binding { ident: cast_ident, res: fhir::Res::GlobalFunc(fhir::SpecFuncKind::Cast) }, + ); rib } diff --git a/tests/tests/pos/surface/resolver11.rs b/tests/tests/pos/surface/resolver11.rs new file mode 100644 index 00000000000..46aced2b735 --- /dev/null +++ b/tests/tests/pos/surface/resolver11.rs @@ -0,0 +1,52 @@ +//! Regression test: an identifier declared and referenced inside the same `macro_rules!` +//! expansion must resolve inside a flux attribute (`#[sig]`), even when it's written as a +//! literal token in the macro body (as opposed to a substituted `$name:ident` metavariable). +#![allow(dead_code)] + +use flux_attrs::*; + +// Case 1: a type parameter declared in the macro's own `impl` header. +struct Wrapper(T); + +macro_rules! wrapper_specs { + ($m:tt) => { + impl Wrapper { + #[sig(fn(x: T) -> T)] + fn identity(x: T) -> T { + x + } + } + }; +} + +wrapper_specs!(dummy); + +// Case 2: an ordinary (non-generic) type declared inside the macro body. +macro_rules! make_stuff { + () => { + struct Foo; + + #[sig(fn(x: Foo) -> Foo)] + fn identity_foo(x: Foo) -> Foo { + x + } + }; +} + +make_stuff!(); + +// Case 3: a type declared inside a macro, then referenced from a flux attribute *outside* the +// macro entirely (ordinary Rust name resolution finds `S` here regardless of hygiene, since +// items introduced by a macro are visible to the enclosing scope like any other item). +macro_rules! declare_s { + () => { + struct S; + }; +} + +declare_s!(); + +#[sig(fn(x: S) -> S)] +fn identity_s(x: S) -> S { + x +} From 52ac5f7ba0a56447c0406a3b68dc5a38b1d3d2e1 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 28 Jul 2026 20:31:09 -0400 Subject: [PATCH 11/12] Fix glob imports clashing with single imports --- crates/flux-desugar/locales/en-US.ftl | 7 + crates/flux-desugar/src/resolver.rs | 601 ++++++++++++++---- .../src/resolver/refinement_resolver.rs | 76 ++- .../resolver/ambiguous_in_module.rs | 42 ++ .../resolver/rust_flux_name_clash.rs | 21 + tests/tests/neg/surface/resolver13.rs | 28 + tests/tests/pos/surface/resolver11.rs | 10 +- tests/tests/pos/surface/resolver12.rs | 91 +++ tests/tests/pos/surface/resolver14.rs | 80 +++ 9 files changed, 807 insertions(+), 149 deletions(-) create mode 100644 tests/tests/neg/error_messages/resolver/ambiguous_in_module.rs create mode 100644 tests/tests/neg/error_messages/resolver/rust_flux_name_clash.rs create mode 100644 tests/tests/neg/surface/resolver13.rs create mode 100644 tests/tests/pos/surface/resolver12.rs create mode 100644 tests/tests/pos/surface/resolver14.rs diff --git a/crates/flux-desugar/locales/en-US.ftl b/crates/flux-desugar/locales/en-US.ftl index 05a84dd3e6d..d25c5d7a003 100644 --- a/crates/flux-desugar/locales/en-US.ftl +++ b/crates/flux-desugar/locales/en-US.ftl @@ -47,6 +47,13 @@ desugar_duplicate_definition = .label = `{$name}` redefined here .previous_definition = previous definition of `{$name}` +desugar_ambiguous_name = + the name `{$name}` is ambiguous + .label = ambiguous name + .note = two different items named `{$name}` are in scope through glob imports + .first_candidate = `{$name}` could refer to the item imported here + .second_candidate = `{$name}` could also refer to the item imported here + desugar_duplicate_param = identifier `{$name}` is bound more than once in this parameter list .label = used as a parameter more than once diff --git a/crates/flux-desugar/src/resolver.rs b/crates/flux-desugar/src/resolver.rs index 1e1743e62e9..a4c4547f043 100644 --- a/crates/flux-desugar/src/resolver.rs +++ b/crates/flux-desugar/src/resolver.rs @@ -35,6 +35,42 @@ use self::refinement_resolver::RefinementResolver; type Result = std::result::Result; +/// The reason a name lookup (`resolve_ident_with_ribs`, `resolve_ident_in_module`, +/// `resolve_path_with_ribs`) failed. +enum ResolveError { + NotFound, + /// The name resolved to two or more distinct, competing bindings (e.g. two different glob + /// imports bringing in different items under the same name) with nothing more specific to + /// disambiguate. Carries everything needed to emit [`errors::AmbiguousName`], so that callers + /// deep in the path walk don't have to reconstruct which segment went wrong. + Ambiguous(Ambiguity), +} + +/// A name that two competing glob imports bind to different items: +/// +/// ```ignore +/// mod a { pub struct S; } +/// mod b { pub struct S; } +/// +/// use a::*; +/// use b::*; +/// +/// #[flux::spec(fn(x: S))] +/// fn f(x: a::S) {} +/// ``` +#[derive(Clone, Copy, Debug)] +struct Ambiguity { + /// The use site that hit the ambiguity: the `S` inside the signature. + span: Span, + /// The contested name: `S`. + name: Symbol, + /// Where the first candidate was brought into scope: the `use a::*;` statement. + first: Span, + /// Same for the second candidate: `use b::*;`. It can coincide with `first` when a single + /// glob imports a module that is itself ambiguously re-exporting the name. + second: Span, +} + pub(crate) fn resolve_crate(genv: GlobalEnv) -> ResolverOutput { match try_resolve_crate(genv) { Ok(output) => output, @@ -82,7 +118,7 @@ impl DefinitionMap { Err(errors::DuplicateDefinition { span: name.span, previous_definition: entry.key().span, - name, + name: name.name, }) } hash_map::Entry::Vacant(entry) => { @@ -172,15 +208,24 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { surface::FluxItem::FuncDef(defn) => { let def_id = FluxDefId::new(parent.def_id.to_def_id(), defn.name.name); let kind = fhir::SpecFuncKind::Def(def_id); - self.define_res_in(defn.name, fhir::Res::GlobalFunc(kind), ReftNS); + self.define_res_in( + fhir::Res::GlobalFunc(kind), + ReftNS, + BindingSource::Explicit(defn.name), + ); } surface::FluxItem::SortDecl(sort_decl) => { let def_id = FluxDefId::new(parent.def_id.to_def_id(), sort_decl.name.name); - self.define_res_in(sort_decl.name, fhir::Res::UserSort(def_id), TypeNS); + self.define_res_in( + fhir::Res::UserSort(def_id), + TypeNS, + BindingSource::Explicit(sort_decl.name), + ); } surface::FluxItem::Use(use_tree) => { + // Flux's `use` has no glob form, so every name it brings in is explicit. for (ident, res, ns) in self.resolve_flux_use_tree(use_tree) { - self.define_res_in(ident, res, ns); + self.define_res_in(res, ns, BindingSource::Explicit(ident)); } } } @@ -197,16 +242,17 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { if let Some(res) = path.res.value_ns && let Ok(res) = fhir::Res::try_from(res) { - self.define_res_in(ident, res, ValueNS); + self.define_res_in(res, ValueNS, BindingSource::Explicit(ident)); } if let Some(res) = path.res.type_ns && let Ok(res) = fhir::Res::try_from(res) { - self.define_res_in(ident, res, TypeNS); + self.define_res_in(res, TypeNS, BindingSource::Explicit(ident)); } } hir::UseKind::Glob => { let is_prelude = is_prelude_import(self.genv.tcx(), item); + let glob_span = item.span; for mod_child in self.glob_imports(path) { if let Ok(res) = fhir::Res::try_from(mod_child.res) && let Some(ns @ (TypeNS | ValueNS)) = res.ns() @@ -214,7 +260,11 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { if is_prelude { self.define_in_prelude(mod_child.ident, res, ns); } else { - self.define_res_in(mod_child.ident, res, ns); + let source = BindingSource::Glob { + ident: mod_child.ident, + glob_span, + }; + self.define_res_in(res, ns, source); } } } @@ -239,7 +289,11 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { if let Some(ns) = def_kind.ns().map(Namespace::from) && let Some(ident) = item.kind.ident() { - self.define_res_in(ident, fhir::Res::Def(def_kind, item.owner_id.to_def_id()), ns); + self.define_res_in( + fhir::Res::Def(def_kind, item.owner_id.to_def_id()), + ns, + BindingSource::Explicit(ident), + ); } } } @@ -250,9 +304,9 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { match item.kind { rustc_hir::ForeignItemKind::Type => { self.define_res_in( - item.ident, fhir::Res::Def(DefKind::ForeignTy, item.owner_id.to_def_id()), TypeNS, + BindingSource::Explicit(item.ident), ); } rustc_hir::ForeignItemKind::Fn(..) | rustc_hir::ForeignItemKind::Static(..) => {} @@ -260,35 +314,48 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } } - /// Define `ident` in the innermost rib of `ns`. If the rib already binds that name, keep the - /// existing binding, report the clash against its original location. - fn define_res_in(&mut self, ident: Ident, res: fhir::Res, ns: Namespace) { + /// Define the name in `source` in the innermost rib of `ns`. + fn define_res_in( + &mut self, + res: fhir::Res, + ns: Namespace, + source: BindingSource, + ) { + let ident = source.ident(); if ident.name == kw::Underscore { return; } - match self.ribs[ns].last_mut().unwrap().bindings.entry(ident.name) { - hash_map::Entry::Occupied(entry) => { - let prev = *entry.get(); - if let fhir::Res::Param(..) = prev.res { - self.emit(errors::DuplicateParam::new(prev.ident, ident)); - } else { - self.emit(errors::DuplicateDefinition { - span: ident.span, - previous_definition: prev.ident.span, - name: ident, - }); - } - } - hash_map::Entry::Vacant(entry) => { - entry.insert(Binding { ident, res }); + let entry = self.ribs[ns] + .last_mut() + .unwrap() + .bindings + .entry(ident.name) + .or_default(); + + if let Some(prev) = entry.define(source, res) { + if let fhir::Res::Param(..) = prev.res { + self.emit(errors::DuplicateParam { + span: ident.span, + name: ident.name, + first_use: prev.span, + }); + } else { + self.emit(errors::DuplicateDefinition { + span: ident.span, + previous_definition: prev.span, + name: ident.name, + }); } - }; + } } + /// Define `ident` in the prelude of `ns`. The prelude is only ever populated from the single + /// `#[prelude_import]` glob, so unlike [`Self::define_res_in`] it keeps plain last-one-wins + /// semantics: there is no second glob to be ambiguous with. fn define_in_prelude(&mut self, ident: Ident, res: fhir::Res, ns: Namespace) { self.prelude[ns] .bindings - .insert(ident.name, Binding { ident, res }); + .insert(ident.name, NameResolution::from_explicit(Binding { span: ident.span, res })); } fn push_rib(&mut self, ns: Namespace, kind: RibKind) { @@ -312,7 +379,11 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { { debug_assert!(matches!(def_kind, DefKind::TyParam | DefKind::ConstParam)); let param_id = self.genv.maybe_extern_id(param.def_id).resolved_id(); - self.define_res_in(name, fhir::Res::Def(def_kind, param_id), ns); + self.define_res_in( + fhir::Res::Def(def_kind, param_id), + ns, + BindingSource::Explicit(name), + ); } } } @@ -351,7 +422,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { &mut self, segments: &[S], ns: Namespace, - ) -> Option> { + ) -> std::result::Result, ResolveError> { let mut module: Option = None; for (segment_idx, segment) in segments.iter().enumerate() { let is_last = segment_idx + 1 == segments.len(); @@ -366,7 +437,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { S::record_segment_res(self, segment, base_res); if is_last { - return Some(fhir::PartialRes::new(base_res)); + return Ok(fhir::PartialRes::new(base_res)); } match base_res { @@ -380,14 +451,14 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { module = Some(Module::new(ModuleKind::Enum, module_id)); } _ => { - return Some(fhir::PartialRes::with_unresolved_segments( + return Ok(fhir::PartialRes::with_unresolved_segments( base_res, segments.len() - segment_idx - 1, )); } } } - None + Err(ResolveError::NotFound) } fn resolve_flux_use_tree( @@ -419,9 +490,16 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } else { self.resolve_ident_with_ribs(ident, TypeNS) }; - let Some(res) = res else { - self.emit_unresolved_import(ident, resolved_module_id, resolved_prefix); - return vec![]; + let res = match res { + Ok(res) => res, + Err(ResolveError::NotFound) => { + self.emit_unresolved_import(ident, resolved_module_id, resolved_prefix); + return vec![]; + } + Err(ResolveError::Ambiguous(ambiguity)) => { + self.emit_ambiguity_err(ambiguity); + return vec![]; + } }; if let fhir::Res::Def(DefKind::Mod, def_id) = res { resolved_module_id = Some(def_id); @@ -435,6 +513,10 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { match &use_tree.kind { surface::UseTreeKind::Simple => { let mut resolutions = vec![]; + // An import names an item in every namespace at once, so an ambiguity in any one + // of them is an error even if the others resolve. Keeping only the first means one + // import is one error. + let mut ambiguity = None; for ns in [TypeNS, ValueNS, ReftNS] { let res = if let Some(module_id) = resolved_module_id { let module = Module::new(ModuleKind::Mod, module_id); @@ -442,11 +524,15 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } else { self.resolve_ident_with_ribs(last.ident(), ns) }; - if let Some(res) = res { - resolutions.push((last.ident(), res, ns)); + match res { + Ok(res) => resolutions.push((last.ident(), res, ns)), + Err(ResolveError::Ambiguous(amb)) => ambiguity = ambiguity.or(Some(amb)), + Err(ResolveError::NotFound) => {} } } - if resolutions.is_empty() { + if let Some(ambiguity) = ambiguity { + self.emit_ambiguity_err(ambiguity); + } else if resolutions.is_empty() { self.emit_unresolved_import(last.ident, resolved_module_id, resolved_prefix); } resolutions @@ -471,11 +557,15 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { &self, ident: Ident, ns: Namespace, - ) -> Option> { + ) -> std::result::Result, ResolveError> { let mut ribs = self.ribs[ns].iter().rev(); while let Some(rib) = ribs.next() { - if let Some(binding) = rib.bindings.get(&ident.name) { - return Some(binding.res); + // An ambiguous name is still a name: it stops the climb here rather than falling + // through to outer scopes or the prelude. + if let Some(name_res) = rib.bindings.get(&ident.name) + && let Some(res) = name_res.resolve(ident) + { + return res.map_err(ResolveError::Ambiguous); } match rib.kind { // A module boundary stops item resolution. @@ -493,23 +583,25 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } if ns == TypeNS { if let Some(crate_id) = self.crates.get(&ident.name) { - return Some(fhir::Res::Def(DefKind::Mod, *crate_id)); + return Ok(fhir::Res::Def(DefKind::Mod, *crate_id)); } // FIXME: `crate` and `super` should only be allowed as the first segment if ident.name == kw::Crate { - return Some(fhir::Res::Def(DefKind::Mod, CRATE_DEF_ID.to_def_id())); + return Ok(fhir::Res::Def(DefKind::Mod, CRATE_DEF_ID.to_def_id())); } if ident.name == kw::Super && let Some(parent) = self.genv.tcx().opt_local_parent(self.current_module.def_id) { - return Some(fhir::Res::Def(DefKind::Mod, parent.to_def_id())); + return Ok(fhir::Res::Def(DefKind::Mod, parent.to_def_id())); } } - if let Some(binding) = self.prelude[ns].bindings.get(&ident.name) { - return Some(binding.res); + if let Some(name_res) = self.prelude[ns].bindings.get(&ident.name) + && let Some(res) = name_res.resolve(ident) + { + return res.map_err(ResolveError::Ambiguous); } - None + Err(ResolveError::NotFound) } fn glob_imports( @@ -522,6 +614,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { let tcx = self.genv.tcx(); let curr_mod = self.current_module.to_def_id(); self.resolve_path_with_ribs(path.segments, TypeNS) + .ok() .and_then(|partial_res| partial_res.full_res()) .and_then(|res| { if let fhir::Res::Def(DefKind::Mod, module_id) = res { @@ -539,42 +632,69 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { module: Module, ident: Ident, ns: Namespace, - ) -> Option> { + ) -> std::result::Result, ResolveError> { let tcx = self.genv.tcx(); - match module.kind { + let res = match module.kind { ModuleKind::Mod => { let module_id = module.def_id; let current_mod = self.current_module.to_def_id(); - // Rust module children take precedence, but are only resolved in a Rust - // namespace. - ns.to_rustc() - .and_then(|rustc_ns| { - visible_module_children(tcx, module_id, current_mod) - .find(|child| { - child.res.matches_ns(rustc_ns) - && tcx.hygienic_eq(ident, child.ident, current_mod) - }) - .and_then(|child| { - fhir::Res::::try_from(child.res).ok() - }) + + // Three sources, tried in order, first one with the name wins: the module's Rust + // children, then the ambiguous ones rustc leaves out of `module_children`, then + // its flux items. Only the first two have a Rust namespace to look in. + // + // Keeping them apart matters because the folds below read every candidate as a + // glob, so one pool would make `mod m { struct Bag; defs! { opaque sort Bag; } }` + // ambiguous at each `m::Bag`. It isn't: two explicit definitions clash as a + // duplicate definition, already reported when `m`'s rib was built. + let mut resolution: NameResolution = ns + .to_rustc() + .into_iter() + .flat_map(|rustc_ns| { + visible_module_children(tcx, module_id, current_mod).filter(move |child| { + child.res.matches_ns(rustc_ns) + && tcx.hygienic_eq(ident, child.ident, current_mod) + }) }) - .or_else(|| { - self.genv - .flux_module_children(module_id) - .iter() - .find(|child| { - child.res.ns() == Some(ns) && child.ident.name == ident.name - }) - .map(|child| child.res.map_param_id(|id| match id {})) + .filter_map(|child| { + Some(Binding { + span: child.ident.span, + res: fhir::Res::try_from(child.res).ok()?, + }) }) + .collect(); + if resolution.is_empty() { + // Still a Rust name, so it comes before any flux item. + if let Some(ambiguity) = self.ambiguous_module_child(module_id, ident, ns) { + return Err(ResolveError::Ambiguous(ambiguity)); + } + resolution = self + .genv + .flux_module_children(module_id) + .iter() + .filter(|child| { + child.res.ns() == Some(ns) && child.ident.name == ident.name + }) + .map(|child| { + Binding { + span: child.ident.span, + res: child.res.map_param_id(|id| match id {}), + } + }) + .collect(); + } + resolution.resolve(ident) } ModuleKind::Trait => { // Associated items are Rust items, so we only ever resolve them in a Rust namespace. - let rustc_ns = ns.to_rustc()?; - let trait_id = module.def_id; - tcx.associated_items(trait_id) - .find_by_ident_and_namespace(tcx, ident, rustc_ns, trait_id) - .map(|assoc| fhir::Res::Def(assoc.kind.as_def_kind(), assoc.def_id)) + ns.to_rustc() + .and_then(|rustc_ns| { + let trait_id = module.def_id; + tcx.associated_items(trait_id) + .find_by_ident_and_namespace(tcx, ident, rustc_ns, trait_id) + .map(|assoc| fhir::Res::Def(assoc.kind.as_def_kind(), assoc.def_id)) + }) + .map(Ok) } ModuleKind::Enum => { tcx.adt_def(module.def_id) @@ -591,15 +711,71 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { }; Some(fhir::Res::Def(kind, def_id)) }) + .map(Ok) } + }; + match res { + Some(res) => res.map_err(ResolveError::Ambiguous), + None => Err(ResolveError::NotFound), } } + /// The ambiguity for `ident` in `module_id`, if the module's *own* glob re-exports bind the + /// name to two different items (`mod m { pub use a::*; pub use b::*; }`). rustc keeps such + /// bindings out of `module_children` — they go into a separate `ambig_module_children` map — + /// so they need their own lookup. + /// + /// Only *local* modules are covered. The data for a foreign module is reachable only through an + /// untracked `CStore` accessor, and rustc doesn't report `E0659` across a crate boundary + /// anyway: it resolves to the first candidate and fires the `ambiguous_glob_imports` + /// future-incompatibility lint instead (see rust-lang/rust#114095). We report those as + /// unresolved. + #[expect(clippy::disallowed_methods, reason = "modules cannot have extern specs")] + fn ambiguous_module_child( + &self, + module_id: DefId, + ident: Ident, + ns: Namespace, + ) -> Option { + let tcx = self.genv.tcx(); + let module_id = module_id.as_local()?; + let rustc_ns = ns.to_rustc()?; + let current_mod = self.current_module.to_def_id(); + let child = tcx + .resolutions(()) + .ambig_module_children + .get(&module_id)? + .iter() + .find(|child| { + child.main.res.matches_ns(rustc_ns) + && tcx.hygienic_eq(ident, child.main.ident, current_mod) + && child.main.vis.is_accessible_from(current_mod, tcx) + })?; + Some(Ambiguity { + span: ident.span, + name: ident.name, + first: self.glob_span(&child.main), + second: self.glob_span(&child.second), + }) + } + + /// Where a module child was brought into its module: the `use ...::*;` statement it was + /// re-exported by, or the item's own definition. Mirrors rustc's `child_span`. + fn glob_span(&self, child: &ModChild) -> Span { + let def_id = child + .reexport_chain + .first() + .and_then(|reexport| reexport.id()) + .unwrap_or_else(|| child.res.def_id()); + self.genv.tcx().def_span(def_id) + } + pub fn into_output(self) -> Result { self.err.into_result()?; Ok(self.output) } + #[track_caller] fn emit_unresolved_import( &mut self, ident: Ident, @@ -614,6 +790,11 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { self.emit(errors::UnresolvedImport { span: ident.span, name, reason }); } + #[track_caller] + fn emit_ambiguity_err(&mut self, ambiguity: Ambiguity) { + self.emit(errors::AmbiguousName::new(ambiguity)); + } + fn emit_not_a_module(&mut self, ident: Ident, resolved_prefix: &[Ident]) { let name = Segment::format_iter(resolved_prefix.iter().chain(&[ident])); self.emit(errors::UnresolvedImport { @@ -623,6 +804,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { }); } + #[track_caller] fn emit(&mut self, err: impl rustc_errors::Diagnostic<'genv>) { self.err.collect(self.genv.sess().emit_err(err)); } @@ -697,20 +879,20 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { ItemKind::Trait(..) => { self.define_generics(def_id); self.define_res_in( - Ident::with_dummy_span(kw::SelfUpper), fhir::Res::SelfTyParam { trait_: def_id.resolved_id() }, TypeNS, + BindingSource::Explicit(Ident::with_dummy_span(kw::SelfUpper)), ); } ItemKind::Impl(hir::Impl { of_trait, .. }) => { self.define_generics(def_id); self.define_res_in( - Ident::with_dummy_span(kw::SelfUpper), fhir::Res::SelfTyAlias { alias_to: def_id.resolved_id(), is_trait_impl: of_trait.is_some(), }, TypeNS, + BindingSource::Explicit(Ident::with_dummy_span(kw::SelfUpper)), ); } ItemKind::TyAlias(..) => { @@ -719,17 +901,17 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { ItemKind::Enum(..) => { self.define_generics(def_id); self.define_res_in( - Ident::with_dummy_span(kw::SelfUpper), fhir::Res::SelfTyAlias { alias_to: def_id.resolved_id(), is_trait_impl: false }, TypeNS, + BindingSource::Explicit(Ident::with_dummy_span(kw::SelfUpper)), ); } ItemKind::Struct(..) => { self.define_generics(def_id); self.define_res_in( - Ident::with_dummy_span(kw::SelfUpper), fhir::Res::SelfTyAlias { alias_to: def_id.resolved_id(), is_trait_impl: false }, TypeNS, + BindingSource::Explicit(Ident::with_dummy_span(kw::SelfUpper)), ); } ItemKind::Fn { .. } => { @@ -819,19 +1001,148 @@ pub(crate) enum RibKind { FnTraitInput, } -/// The value stored for each binding in a [`Rib`]. `ident` is the original identifier (kept for -/// its span, used to point at the previous location on a duplicate-definition clash); lookups -/// and clash detection are keyed by `ident.name` alone. +/// The value stored for each binding in a [`Rib`]. Lookups and clash detection are keyed by the +/// name in the [`Rib`]'s map, so all a binding carries is where it came from, for diagnostics that +/// point at the previous location of a name. #[derive(Clone, Copy, Debug)] struct Binding { - ident: Ident, + /// Where the name was brought into scope: the item, generic or param itself, or, for a + /// glob-imported binding, the `use ...::*;` statement rather than the imported item's + /// declaration, matching where rustc points its `E0659` notes. + span: Span, res: fhir::Res, } +/// How a name was brought into a rib. Explicit definitions (items, generics, params, single +/// `use`s, flux `use`s) shadow glob imports of the same name in the same namespace, regardless of +/// declaration order; only globs can be ambiguous with each other. +#[derive(Clone, Copy, Debug)] +enum BindingSource { + /// A name written where it is defined: an item, generic, param or single `use`. + Explicit(Ident), + /// A glob-imported name: the imported item's ident, plus the span of the `use ...::*;` + /// statement that brought it in, which is the one diagnostics cite. + Glob { ident: Ident, glob_span: Span }, +} + +impl BindingSource { + fn ident(self) -> Ident { + match self { + BindingSource::Explicit(ident) | BindingSource::Glob { ident, .. } => ident, + } + } + + /// Where the name was brought into scope. + fn span(self) -> Span { + match self { + BindingSource::Explicit(ident) => ident.span, + BindingSource::Glob { glob_span, .. } => glob_span, + } + } +} + +/// The glob-imported candidate(s) for a name, mirroring rustc's `glob_decl` slot. +#[derive(Clone, Copy, Debug)] +enum GlobBinding { + Single(Binding), + /// Two competing globs bound this name to different items. Frozen once set: further + /// candidates never change it. We deliberately diverge from rustc here, which keeps updating + /// the second span as new distinct competitors show up; with three or more globs we therefore + /// report the first two rather than the first and the last. + Ambiguous(Span, Span), +} + +/// All candidates for a single name in a single namespace, split into the two slots +/// (`non_glob_decl`/`glob_decl`). The non-glob slot always wins. +#[derive(Clone, Copy, Debug, Default)] +struct NameResolution { + non_glob: Option, + glob: Option, +} + +impl NameResolution { + fn from_explicit(binding: Binding) -> Self { + Self { non_glob: Some(binding), glob: None } + } + + fn is_empty(&self) -> bool { + self.non_glob.is_none() && self.glob.is_none() + } + + /// Define the binding. Return previously defined binding if there's a clash + fn define( + &mut self, + source: BindingSource, + res: fhir::Res, + ) -> Option { + let binding = Binding { span: source.span(), res }; + match source { + BindingSource::Explicit(_) => { + if let Some(prev) = self.non_glob { + Some(prev) + } else { + self.non_glob = Some(binding); + None + } + } + BindingSource::Glob { .. } => { + self.add_glob(binding); + None + } + } + } + + /// Fold one more glob candidate into the glob slot. A candidate resolving to the same item as + /// the one already recorded (the same item reached through two different globs) is not an + /// ambiguity. + fn add_glob(&mut self, binding: Binding) { + self.glob = Some(match self.glob { + None => GlobBinding::Single(binding), + Some(GlobBinding::Single(prev)) => { + if prev.res == binding.res { + GlobBinding::Single(prev) + } else { + GlobBinding::Ambiguous(prev.span, binding.span) + } + } + Some(ambiguous @ GlobBinding::Ambiguous(..)) => ambiguous, + }); + } + + /// The resolution for `ident`, or `None` if this holds no candidate at all (in which case the + /// caller should keep looking in outer scopes). + fn resolve( + &self, + ident: Ident, + ) -> Option, Ambiguity>> { + if let Some(binding) = self.non_glob { + return Some(Ok(binding.res)); + } + match self.glob? { + GlobBinding::Single(binding) => Some(Ok(binding.res)), + GlobBinding::Ambiguous(first, second) => { + Some(Err(Ambiguity { span: ident.span, name: ident.name, first, second })) + } + } + } +} + +impl FromIterator for NameResolution { + /// Fold a stream of *glob* candidates. Used to merge a module's children, where two entries + /// for the same name can only come from an ambiguous glob re-export. + fn from_iter>(iter: T) -> Self { + let mut resolution = Self::default(); + for binding in iter { + resolution.add_glob(binding); + } + resolution + } +} + #[derive(Debug)] struct Rib { kind: RibKind, - bindings: UnordMap, + bindings: UnordMap, } impl Rib { @@ -968,13 +1279,15 @@ impl<'a, 'genv, 'tcx> ItemResolver<'a, 'genv, 'tcx> { } fn resolve_path_in(&mut self, ns: Namespace, path: &surface::Path) { - if let Some(partial_res) = self.resolver.resolve_path_with_ribs(&path.segments, ns) { - self.resolver - .output - .path_res_map - .insert(path.node_id, partial_res); - } else { - self.emit_unresolved_path(path, ns); + match self.resolver.resolve_path_with_ribs(&path.segments, ns) { + Ok(partial_res) => { + self.resolver + .output + .path_res_map + .insert(path.node_id, partial_res); + } + Err(ResolveError::NotFound) => self.emit_unresolved_path(path, ns), + Err(ResolveError::Ambiguous(ambiguity)) => self.emit_ambiguity_err(ambiguity), } } @@ -1031,19 +1344,29 @@ impl<'a, 'genv, 'tcx> ItemResolver<'a, 'genv, 'tcx> { fn resolve_reveals(&mut self, item_id: surface::NodeId, reveal_names: &[Ident]) { let mut reveals = Vec::with_capacity(reveal_names.len()); for reveal in reveal_names { - if let Some(fhir::Res::GlobalFunc(kind)) = - self.resolver.resolve_ident_with_ribs(*reveal, ReftNS) - && let Some(def_id) = kind.def_id() - { - reveals.push(def_id); - } else { - self.errors - .emit(errors::UnknownRevealDefinition::new(reveal.span)); + match self.resolver.resolve_ident_with_ribs(*reveal, ReftNS) { + Ok(fhir::Res::GlobalFunc(kind)) => { + if let Some(def_id) = kind.def_id() { + reveals.push(def_id); + } else { + self.errors + .emit(errors::UnknownRevealDefinition::new(reveal.span)); + } + } + Ok(_) | Err(ResolveError::NotFound) => { + self.errors + .emit(errors::UnknownRevealDefinition::new(reveal.span)); + } + Err(ResolveError::Ambiguous(ambiguity)) => self.emit_ambiguity_err(ambiguity), } } self.resolver.output.reveal_res_map.insert(item_id, reveals); } + fn emit_ambiguity_err(&mut self, ambiguity: Ambiguity) { + self.errors.emit(errors::AmbiguousName::new(ambiguity)); + } + fn emit_unresolved_path(&mut self, path: &surface::Path, ns: Namespace) { self.errors.emit(errors::UnresolvedName { span: path.span, @@ -1093,10 +1416,14 @@ impl surface::visit::Visitor for ItemResolver<'_, '_, '_> { // parsing. We try to resolve that ambiguity by attempting resolution in both the // type and value namespaces. If we resolved the path in the value namespace, we // transform it into a generic const argument. + // A name that is *ambiguous* in a namespace still counts as present there: rerouting + // to the other namespace would hide the ambiguity instead of reporting it. The real + // diagnostic comes from `resolve_path_in` below. let check_ns = |ns| { - self.resolver - .resolve_ident_with_ribs(path.last().ident, ns) - .is_some() + !matches!( + self.resolver.resolve_ident_with_ribs(path.last().ident, ns), + Err(ResolveError::NotFound) + ) }; if !check_ns(TypeNS) && check_ns(ValueNS) { @@ -1128,14 +1455,28 @@ fn builtin_types_rib() -> Rib { use flux_middle::fhir::PrimSort; let sorts = PrimSort::ALL.into_iter().map(|prim| { let ident = Ident::with_dummy_span(prim.name()); - (ident.name, Binding { ident, res: fhir::Res::PrimSort(prim) }) + ( + ident.name, + NameResolution::from_explicit(Binding { + span: ident.span, + res: fhir::Res::PrimSort(prim), + }), + ) }); let types = PrimTy::ALL.into_iter().map(|pty| { let ident = Ident::with_dummy_span(pty.name()); - (ident.name, Binding { ident, res: fhir::Res::PrimTy(pty) }) + ( + ident.name, + NameResolution::from_explicit(Binding { + span: ident.span, + res: fhir::Res::PrimTy(pty), + }), + ) }); - // Types go after such that they override sorts with the same name + // Types go after such that they override sorts with the same name. Note this collects into a + // map, so it is a plain last-one-wins overwrite, not the shadowing/ambiguity fold used for + // user-written definitions. let bindings = sorts.chain(types).collect(); Rib { kind: RibKind::Misc, bindings } } @@ -1147,12 +1488,15 @@ fn theory_funcs_rib() -> Rib { .extend_unord(flux_middle::THEORY_FUNCS.items().map(|(_, itf)| { let ident = Ident::with_dummy_span(itf.name); let res = fhir::Res::GlobalFunc(fhir::SpecFuncKind::Thy(itf.itf)); - (ident.name, Binding { ident, res }) + (ident.name, NameResolution::from_explicit(Binding { span: ident.span, res })) })); let cast_ident = Ident::with_dummy_span(sym::cast); rib.bindings.insert( cast_ident.name, - Binding { ident: cast_ident, res: fhir::Res::GlobalFunc(fhir::SpecFuncKind::Cast) }, + NameResolution::from_explicit(Binding { + span: cast_ident.span, + res: fhir::Res::GlobalFunc(fhir::SpecFuncKind::Cast), + }), ); rib } @@ -1173,7 +1517,7 @@ fn mk_crate_mapping(tcx: TyCtxt) -> UnordMap { mod errors { use flux_errors::E0999; use flux_macros::Diagnostic; - use rustc_span::{Ident, Span, Symbol}; + use rustc_span::{Span, Symbol}; /// A name that could not be resolved. `kind` is the user-facing description of what was being /// looked for (`"type"`, `"value"`, `"sort"`, ...); it is passed explicitly by each call site @@ -1250,24 +1594,43 @@ mod errors { pub span: Span, #[label(desugar_previous_definition)] pub previous_definition: Span, - pub name: Ident, + pub name: Symbol, } + /// A name bound to two different items by two competing glob imports, reported at the first + /// use of the name (imports themselves are never an error). Mirrors rustc's `E0659`. #[derive(Diagnostic)] - #[diag(desugar_duplicate_param, code = E0999)] - pub(super) struct DuplicateParam { + #[diag(desugar_ambiguous_name, code = E0999)] + #[note] + pub(super) struct AmbiguousName { #[primary_span] #[label] span: Span, name: Symbol, - #[label(desugar_first_use)] - first_use: Span, + #[label(desugar_first_candidate)] + first: Span, + #[label(desugar_second_candidate)] + second: Option, + } + + impl AmbiguousName { + pub(super) fn new(ambiguity: super::Ambiguity) -> Self { + let super::Ambiguity { span, name, first, second } = ambiguity; + // Both candidates can come from the same glob statement, when it imports a module + // that is itself ambiguously glob re-exporting the name. Pointing a second, identical + // label at it reads as a bug, so drop it. + Self { span, name, first, second: (first != second).then_some(second) } + } } - impl DuplicateParam { - pub(super) fn new(old_ident: Ident, new_ident: Ident) -> Self { - debug_assert_eq!(old_ident.name, new_ident.name); - Self { span: new_ident.span, name: new_ident.name, first_use: old_ident.span } - } + #[derive(Diagnostic)] + #[diag(desugar_duplicate_param, code = E0999)] + pub(super) struct DuplicateParam { + #[primary_span] + #[label] + pub span: Span, + pub name: Symbol, + #[label(desugar_first_use)] + pub first_use: Span, } } diff --git a/crates/flux-desugar/src/resolver/refinement_resolver.rs b/crates/flux-desugar/src/resolver/refinement_resolver.rs index 6eb03a7b5b4..a56d0f03099 100644 --- a/crates/flux-desugar/src/resolver/refinement_resolver.rs +++ b/crates/flux-desugar/src/resolver/refinement_resolver.rs @@ -19,7 +19,7 @@ use rustc_hash::FxHashMap; use rustc_middle::ty::TyCtxt; use rustc_span::{ErrorGuaranteed, Span}; -use super::{CrateResolver, RibKind, Segment}; +use super::{BindingSource, CrateResolver, ResolveError, RibKind, Segment}; type Result = std::result::Result; @@ -405,8 +405,11 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { // Sort variables share the type namespace with sorts/types (see [`fhir::Namespace`]). self.resolver.push_rib(TypeNS, RibKind::Misc); for (idx, ident) in sort_vars.iter().enumerate() { - self.resolver - .define_res_in(*ident, Res::SortParam(idx), TypeNS); + self.resolver.define_res_in( + Res::SortParam(idx), + TypeNS, + BindingSource::Explicit(*ident), + ); } let mut wrapper = self.wrap(); f(&mut wrapper); @@ -425,27 +428,38 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { self.param_defs .insert(param_id, ParamDef { ident, kind, scope }); - self.resolver - .define_res_in(ident, Res::Param(kind, param_id), ReftNS); + self.resolver.define_res_in( + Res::Param(kind, param_id), + ReftNS, + BindingSource::Explicit(ident), + ); + } + + fn emit_ambiguity_err(&mut self, ambiguity: super::Ambiguity) { + self.errors + .emit(super::errors::AmbiguousName::new(ambiguity)); } fn resolve_path(&mut self, path: &surface::ExprPath) { - if let Some(res) = self.try_resolve_expr_with_ribs(&path.segments) { - self.check_unrefined_param(res, path.segments.last().unwrap().ident); - self.path_res_map.insert(path.node_id, res); - return; + match self.try_resolve_expr_with_ribs(&path.segments) { + Ok(res) => { + self.check_unrefined_param(res, path.segments.last().unwrap().ident); + self.path_res_map.insert(path.node_id, res); + } + Err(ResolveError::Ambiguous(ambiguity)) => self.emit_ambiguity_err(ambiguity), + Err(ResolveError::NotFound) => self.emit_unresolved_expr_path(path), } - - self.emit_unresolved_expr_path(path); } fn resolve_ident(&mut self, ident: Ident, node_id: NodeId) { - if let Some(res) = self.try_resolve_expr_with_ribs(&[ident]) { - self.check_unrefined_param(res, ident); - self.path_res_map.insert(node_id, res); - return; + match self.try_resolve_expr_with_ribs(&[ident]) { + Ok(res) => { + self.check_unrefined_param(res, ident); + self.path_res_map.insert(node_id, res); + } + Err(ResolveError::Ambiguous(ambiguity)) => self.emit_ambiguity_err(ambiguity), + Err(ResolveError::NotFound) => self.emit_unresolved_ident(ident), } - self.emit_unresolved_ident(ident); } /// Emit an error if `res` resolved to a param that cannot be refined. @@ -459,16 +473,24 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { fn try_resolve_expr_with_ribs( &mut self, segments: &[S], - ) -> Option> { + ) -> std::result::Result, ResolveError> { // Try the refinement namespace first so that refinement params (and then flux funcs) take // precedence over Rust value/type bindings — in particular, a param shadows a Rust const of // the same name. + // + // An ambiguity in one namespace doesn't stop us from trying the next: a later namespace + // may still resolve the name unambiguously, exactly as it would shadow it. But if nothing + // resolves anywhere, we report the ambiguity rather than "unresolved name", which would + // point the user at the wrong problem. + let mut ambiguity = None; for ns in [ReftNS, ValueNS, TypeNS] { - if let Some(partial_res) = self.resolver.resolve_path_with_ribs(segments, ns) { - return Some(partial_res); + match self.resolver.resolve_path_with_ribs(segments, ns) { + Ok(partial_res) => return Ok(partial_res), + Err(ResolveError::Ambiguous(amb)) => ambiguity = ambiguity.or(Some(amb)), + Err(ResolveError::NotFound) => {} } } - None + Err(ambiguity.map_or(ResolveError::NotFound, ResolveError::Ambiguous)) } fn resolve_sort_path(&mut self, path: &surface::SortPath) { @@ -476,13 +498,17 @@ impl<'a, 'genv, 'tcx> RefinementResolver<'a, 'genv, 'tcx> { // alongside types, and any type can also denote a sort. We only report a *name* that fails // to resolve here; whether the resolved item is admissible as a sort (and the corresponding // diagnostic) is decided later in `conv_sort_path`. - let res = self - .resolver - .resolve_path_with_ribs(&path.segments, TypeNS) - .unwrap_or_else(|| { + let res = match self.resolver.resolve_path_with_ribs(&path.segments, TypeNS) { + Ok(res) => res, + Err(ResolveError::NotFound) => { self.emit_unresolved_sort_path(path); PartialRes::new(fhir::Res::Err) - }); + } + Err(ResolveError::Ambiguous(ambiguity)) => { + self.emit_ambiguity_err(ambiguity); + PartialRes::new(fhir::Res::Err) + } + }; self.resolver.output.path_res_map.insert(path.node_id, res); } diff --git a/tests/tests/neg/error_messages/resolver/ambiguous_in_module.rs b/tests/tests/neg/error_messages/resolver/ambiguous_in_module.rs new file mode 100644 index 00000000000..0332e9439cf --- /dev/null +++ b/tests/tests/neg/error_messages/resolver/ambiguous_in_module.rs @@ -0,0 +1,42 @@ +//! A *qualified* path into a module whose own glob re-exports bind the same name to two different +//! items (`mod m { pub use a::*; pub use b::*; }` and then `m::S`) is ambiguous, just like the +//! unqualified case in `resolver13.rs`. rustc reports `E0659` here too. +//! +//! `S` is only ever named from a flux `use`, so rustc never resolves it and reports nothing; +//! detecting the ambiguity is entirely up to us. +#![allow(dead_code, unused_imports, non_snake_case)] + +use flux_attrs::*; + +mod a { + pub struct S {} +} + +mod b { + pub struct S {} +} + +mod m { + pub use super::{a::*, b::*}; +} + +defs! { + use m::S; //~ ERROR the name `S` is ambiguous +} + +// The same ambiguity reached through a qualified path in a signature rather than a flux `use`. +#[spec(fn(x: m::S))] //~ ERROR the name `S` is ambiguous +fn test(_x: i32) {} + +// An item defined in `m` itself shadows the globs, but only in *its* namespace: `S` is still +// ambiguous in the type namespace. rustc reports the import too, even though the value namespace +// resolves cleanly to `m::S` the function. +mod shadowed { + pub use super::{a::*, b::*}; + + pub fn S() {} +} + +defs! { + use shadowed::S; //~ ERROR the name `S` is ambiguous +} diff --git a/tests/tests/neg/error_messages/resolver/rust_flux_name_clash.rs b/tests/tests/neg/error_messages/resolver/rust_flux_name_clash.rs new file mode 100644 index 00000000000..b6b23a34b38 --- /dev/null +++ b/tests/tests/neg/error_messages/resolver/rust_flux_name_clash.rs @@ -0,0 +1,21 @@ +//! A Rust item and a flux item of the same name in the same module are two explicit definitions, +//! so the clash is a duplicate definition, reported once where they are defined. Resolving the +//! name from outside must not report it again as an ambiguity: `resolve_ident_in_module` keeps the +//! Rust and flux children in separate pools, and the Rust one wins. +#![allow(dead_code)] + +use flux_attrs::*; + +mod m { + use flux_attrs::*; + + pub struct Bag; + + defs! { + opaque sort Bag; //~ ERROR name `Bag` is defined multiple times + } +} + +// Naming `Bag` through a qualified path resolves to the Rust item and adds no error of its own. +#[spec(fn(x: m::Bag))] +fn test(_x: m::Bag) {} diff --git a/tests/tests/neg/surface/resolver13.rs b/tests/tests/neg/surface/resolver13.rs new file mode 100644 index 00000000000..579b42ad5e7 --- /dev/null +++ b/tests/tests/neg/surface/resolver13.rs @@ -0,0 +1,28 @@ +//! Two competing glob imports binding the same name to two *different* items are not an error at +//! import time; the ambiguity is deferred to the first use of the name, matching rustc's `E0659` +//! (). +//! +//! `S` is only ever mentioned inside a flux annotation, so rustc never resolves it and reports +//! nothing here — detecting the ambiguity is entirely up to us. +//! +//! The *qualified* form of the same thing — a path into a module whose own glob re-exports are +//! ambiguous (`mod m { pub use a::*; pub use b::*; }` and then `m::S`) — is covered by +//! `neg/error_messages/resolver/ambiguous_in_module.rs`. +#![allow(unused_imports, dead_code)] + +mod a { + pub struct S; +} + +mod b { + pub struct S; +} + +mod two_globs { + use super::{a::*, b::*}; + + #[flux::sig(fn(x: S) -> i32)] //~ ERROR the name `S` is ambiguous + fn test(x: i32) -> i32 { + x + } +} diff --git a/tests/tests/pos/surface/resolver11.rs b/tests/tests/pos/surface/resolver11.rs index 46aced2b735..3f98e3b0ea4 100644 --- a/tests/tests/pos/surface/resolver11.rs +++ b/tests/tests/pos/surface/resolver11.rs @@ -1,5 +1,5 @@ -//! Regression test: an identifier declared and referenced inside the same `macro_rules!` -//! expansion must resolve inside a flux attribute (`#[sig]`), even when it's written as a +//! An identifier declared and referenced inside the same `macro_rules!` +//! expansion must resolve inside a flux attribute (`#[spec]`), even when it's written as a //! literal token in the macro body (as opposed to a substituted `$name:ident` metavariable). #![allow(dead_code)] @@ -11,7 +11,7 @@ struct Wrapper(T); macro_rules! wrapper_specs { ($m:tt) => { impl Wrapper { - #[sig(fn(x: T) -> T)] + #[spec(fn(x: T) -> T)] fn identity(x: T) -> T { x } @@ -26,7 +26,7 @@ macro_rules! make_stuff { () => { struct Foo; - #[sig(fn(x: Foo) -> Foo)] + #[spec(fn(x: Foo) -> Foo)] fn identity_foo(x: Foo) -> Foo { x } @@ -46,7 +46,7 @@ macro_rules! declare_s { declare_s!(); -#[sig(fn(x: S) -> S)] +#[spec(fn(x: S) -> S)] fn identity_s(x: S) -> S { x } diff --git a/tests/tests/pos/surface/resolver12.rs b/tests/tests/pos/surface/resolver12.rs new file mode 100644 index 00000000000..b4e1f657fb2 --- /dev/null +++ b/tests/tests/pos/surface/resolver12.rs @@ -0,0 +1,91 @@ +//! An explicit import must silently shadow a glob import that brings in a +//! same-named (but distinct) item, matching real Rust's shadowing rules +//! (: "Items and named imports +//! are allowed to shadow names from glob imports in the same namespace"). This must not be +//! reported as a duplicate definition, and the explicit import must be the one actually used. +#![allow(dead_code)] + +use flux_attrs::*; + +// Case 0: plain Rust items, no flux attributes at all, matching the shape of the original bug +// report (`use crate::debug;` colliding with a glob-imported, unrelated `debug` re-export). +mod plain_a { + pub fn debug() {} +} + +mod plain_b { + pub fn debug() {} +} + +use plain_a::debug; +use plain_b::*; + +fn test_plain() { + debug(); +} + +// Case 1: explicit import vs. a glob bringing in an unrelated, same-named item, this time +// through flux-attributed functions so we can also verify *which* definition resolution picked. +mod a { + use flux_attrs::*; + + #[spec(fn(x: i32) -> i32[x + 1])] + pub fn shift(x: i32) -> i32 { + x + 1 + } +} + +mod b { + use flux_attrs::*; + + #[spec(fn(x: i32) -> i32[x + 2])] + pub fn shift(x: i32) -> i32 { + x + 2 + } +} + +use a::shift; +use b::*; + +// If resolution had picked up `b::shift` (or errored as a duplicate) instead of the explicit +// `a::shift`, this postcondition (`x + 1`, not `x + 2`) would fail to verify. +#[spec(fn(x: i32) -> i32[x + 1])] +pub fn test_explicit_wins(x: i32) -> i32 { + shift(x) +} + +// Case 2: same shadowing rule, but the glob brings the name in transitively through a +// re-export, mirroring the shape that originally surfaced this bug (an explicit `use` for a +// local item colliding with a glob-imported re-export of an unrelated item from elsewhere). +mod inner { + use flux_attrs::*; + + #[spec(fn(x: i32) -> i32[x + 3])] + pub fn dbl(x: i32) -> i32 { + x + 3 + } +} + +mod reexport { + pub use super::inner::dbl; +} + +mod local { + use flux_attrs::*; + + #[spec(fn(x: i32) -> i32[2 * x])] + pub fn dbl(x: i32) -> i32 { + 2 * x + } +} + +mod use_local { + use flux_attrs::*; + + use crate::{local::dbl, reexport::*}; + + #[spec(fn(x: i32) -> i32[2 * x])] + pub fn test_explicit_wins_reexport(x: i32) -> i32 { + dbl(x) + } +} diff --git a/tests/tests/pos/surface/resolver14.rs b/tests/tests/pos/surface/resolver14.rs new file mode 100644 index 00000000000..e945b64a3e7 --- /dev/null +++ b/tests/tests/pos/surface/resolver14.rs @@ -0,0 +1,80 @@ +//! Companion to `neg/surface/resolver13.rs`: the cases where competing glob imports must *not* be +//! reported as ambiguous. +#![allow(dead_code, unused_imports)] + +// Case 0: two globs bringing in the *same* item through different paths. Not a conflict, in flux +// or in rustc. +mod inner { + pub struct S; +} + +mod re1 { + pub use super::inner::S; +} + +mod re2 { + pub use super::inner::S; +} + +mod same_item_twice { + use super::{re1::*, re2::*}; + + #[flux::sig(fn(x: S))] + fn test(x: S) {} +} + +// Case 1: an explicit import shadows a glob import of a different item with the same name, in +// *either* declaration order. `resolver12.rs` covers explicit-first; this is glob-first, which +// order-dependent shadowing would get wrong. +mod a { + #[flux::sig(fn(x: i32) -> i32[x + 1])] + pub fn shift(x: i32) -> i32 { + x + 1 + } +} + +mod b { + #[flux::sig(fn(x: i32) -> i32[x + 2])] + pub fn shift(x: i32) -> i32 { + x + 2 + } +} + +mod glob_first { + use super::b::*; + + use super::a::shift; + + // `x + 1` (from the explicit `a::shift`), not `x + 2`. + #[flux::sig(fn(x: i32) -> i32[x + 1])] + pub fn test(x: i32) -> i32 { + shift(x) + } +} + +// Case 2: a locally defined item shadows a glob import of a same-named item, and the local one is +// what flux paths resolve to. +mod other { + #[flux::refined_by(n: int)] + pub struct S { + #[flux::field(i32[n])] + pub val: i32, + } +} + +mod local_item_wins { + use super::other::*; + + // Shadows the glob-imported `other::S`, which is indexed by an `int` rather than a `bool`, so + // the signature below would not even sort-check if resolution picked the wrong one. + #[flux::refined_by(b: bool)] + pub struct S { + #[flux::field(bool[b])] + pub flag: bool, + } + + #[flux::sig(fn(x: S[true]) -> bool[true])] + pub fn test(x: S) -> bool { + x.flag + } +} From 4b5eb4346a470305a0b246b0f28c8ae3c7ab72b9 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Wed, 29 Jul 2026 20:45:20 -0400 Subject: [PATCH 12/12] Add test for flux-core --- lib/flux-core/src/lib.rs | 2 +- .../pos/surface/use_flux_core_defs.rs | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 tests/tests/with_deps/pos/surface/use_flux_core_defs.rs diff --git a/lib/flux-core/src/lib.rs b/lib/flux-core/src/lib.rs index f8b5365a082..9ae46a145d3 100644 --- a/lib/flux-core/src/lib.rs +++ b/lib/flux-core/src/lib.rs @@ -28,7 +28,7 @@ mod slice; mod array; #[cfg(flux)] -mod num; +pub mod num; #[cfg(flux)] mod ptr; diff --git a/tests/tests/with_deps/pos/surface/use_flux_core_defs.rs b/tests/tests/with_deps/pos/surface/use_flux_core_defs.rs new file mode 100644 index 00000000000..240ac331171 --- /dev/null +++ b/tests/tests/with_deps/pos/surface/use_flux_core_defs.rs @@ -0,0 +1,41 @@ +//! Import a flux def from flux-core +#![allow(dead_code)] + +extern crate flux_core; + +use flux_attrs::*; + +defs! { + use flux_core::num::clamp; +} + +#[spec(fn(x: i32) -> i32[clamp(x, 0, 10)])] +fn clamp_to_range(x: i32) -> i32 { + if x < 0 { + 0 + } else if x > 10 { + 10 + } else { + x + } +} + +// A qualified path to the same def resolves to what the import brought in. +#[spec(fn(x: i32{0 <= x && x < 10}) -> i32[flux_core::num::clamp(x, 0, 10)])] +fn already_in_range(x: i32) -> i32 { + x +} + +// The def is importable again from a nested module. +mod inner { + use flux_attrs::*; + + defs! { + use flux_core::num::clamp; + } + + #[spec(fn(x: i32{x > 10}) -> i32[clamp(x, 0, 10)])] + fn above_range(_x: i32) -> i32 { + 10 + } +}