From 3b9ea969e49217c3dba7ed3fec68c0d19ab2b277 Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 28 Jan 2025 15:43:38 -0800 Subject: [PATCH] Support fully qualified syntax in refinements (#976) --- crates/flux-desugar/src/resolver.rs | 141 ++++++++++-------- crates/flux-fhir-analysis/locales/en-US.ftl | 4 + crates/flux-fhir-analysis/src/conv/mod.rs | 66 +++++--- crates/flux-fhir-analysis/src/lib.rs | 2 +- .../error_messages/conv/ambiguous_assoc_ty.rs | 10 ++ .../neg/error_messages/conv/expected_type.rs | 10 ++ 6 files changed, 152 insertions(+), 81 deletions(-) create mode 100644 tests/tests/neg/error_messages/conv/ambiguous_assoc_ty.rs create mode 100644 tests/tests/neg/error_messages/conv/expected_type.rs diff --git a/crates/flux-desugar/src/resolver.rs b/crates/flux-desugar/src/resolver.rs index a02f81a7f5..d87c94417e 100644 --- a/crates/flux-desugar/src/resolver.rs +++ b/crates/flux-desugar/src/resolver.rs @@ -5,11 +5,7 @@ use flux_common::{ result::{ErrorCollector, ResultExt}, }; use flux_errors::{Errors, FluxSession}; -use flux_middle::{ - fhir::{self, Res}, - global_env::GlobalEnv, - MaybeExternId, ResolverOutput, Specs, -}; +use flux_middle::{fhir, global_env::GlobalEnv, MaybeExternId, ResolverOutput, Specs}; use flux_syntax::surface::{self, visit::Visitor as _, Ident}; use hir::{def::DefKind, ItemId, ItemKind, OwnerId}; use rustc_data_structures::unord::{ExtendUnord, UnordMap}; @@ -121,17 +117,20 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { hir::UseKind::Single => { let name = path.segments.last().unwrap().ident.name; for res in &path.res { - if let Some(ns @ (TypeNS | ValueNS)) = res.ns() { - self.define_res_in(name, *res, ns); + if let Some(ns @ (TypeNS | ValueNS)) = res.ns() + && let Ok(res) = fhir::Res::try_from(*res) + { + self.define_res_in(name, res, ns); } } } hir::UseKind::Glob => { let is_prelude = is_prelude_import(self.genv.tcx(), item); for mod_child in self.glob_imports(path) { - if let Some(ns @ (TypeNS | ValueNS)) = mod_child.res.ns() { + if let Some(ns @ (TypeNS | ValueNS)) = mod_child.res.ns() + && let Ok(res) = fhir::Res::try_from(mod_child.res) + { let name = mod_child.ident.name; - let res = map_res(mod_child.res); if is_prelude { self.define_in_prelude(name, res, ns); } else { @@ -155,18 +154,18 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { if let Some(ns) = def_kind.ns() { self.define_res_in( item.ident.name, - hir::def::Res::Def(def_kind, item.owner_id.to_def_id()), + fhir::Res::Def(def_kind, item.owner_id.to_def_id()), ns, ); } } } - fn define_res_in(&mut self, name: Symbol, res: hir::def::Res, ns: Namespace) { + fn define_res_in(&mut self, name: Symbol, res: fhir::Res, ns: Namespace) { self.ribs[ns].last_mut().unwrap().bindings.insert(name, res); } - fn define_in_prelude(&mut self, name: Symbol, res: hir::def::Res, ns: Namespace) { + fn define_in_prelude(&mut self, name: Symbol, res: fhir::Res, ns: Namespace) { self.prelude[ns].bindings.insert(name, res); } @@ -191,7 +190,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, hir::def::Res::Def(def_kind, param_id), ns); + self.define_res_in(name.name, fhir::Res::Def(def_kind, param_id), ns); } } } @@ -275,35 +274,43 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { segments: &[S], ns: Namespace, ) -> Option { - let mut module: Option = None; + let mut module: Option = None; for (segment_idx, segment) in segments.iter().enumerate() { let is_last = segment_idx + 1 == segments.len(); let ns = if is_last { ns } else { TypeNS }; - let res = if let Some(module) = module { - self.resolve_ident_in_module(module, segment.ident())? + 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)? }; - let base_res = Res::try_from(res).ok()?; - S::record_segment_res(self, segment, base_res); - if let Res::Def(DefKind::Mod, module_id) = base_res { - module = Some(module_id); - } else { - return Some(fhir::PartialRes::with_unresolved_segments( - base_res, - segments.len() - segment_idx - 1, - )); + if is_last { + return Some(fhir::PartialRes::new(base_res)); + } + + match base_res { + fhir::Res::Def(DefKind::Mod, module_id) => { + module = Some(Module::new(ModuleKind::Mod, module_id)); + } + fhir::Res::Def(DefKind::Trait, module_id) => { + module = Some(Module::new(ModuleKind::Trait, module_id)); + } + _ => { + return Some(fhir::PartialRes::with_unresolved_segments( + base_res, + segments.len() - segment_idx - 1, + )); + } } } None } - fn resolve_ident_with_ribs(&self, ident: Ident, ns: Namespace) -> Option { + fn resolve_ident_with_ribs(&self, ident: Ident, ns: Namespace) -> Option { for rib in self.ribs[ns].iter().rev() { if let Some(res) = rib.bindings.get(&ident.name) { return Some(*res); @@ -314,7 +321,7 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { } if ns == TypeNS { if let Some(crate_id) = self.crates.get(&ident.name) { - return Some(hir::def::Res::Def(DefKind::Mod, *crate_id)); + return Some(fhir::Res::Def(DefKind::Mod, *crate_id)); } } if let Some(res) = self.prelude[ns].bindings.get(&ident.name) { @@ -333,9 +340,27 @@ impl<'genv, 'tcx> CrateResolver<'genv, 'tcx> { .flat_map(move |module_id| visible_module_children(tcx, module_id, curr_mod)) } - fn resolve_ident_in_module(&self, module_id: DefId, ident: Ident) -> Option { - visible_module_children(self.genv.tcx(), module_id, self.current_module.to_def_id()) - .find_map(|child| if child.ident == ident { Some(map_res(child.res)) } else { None }) + fn resolve_ident_in_module( + &self, + module: &Module, + ident: Ident, + ns: Namespace, + ) -> Option { + match module.kind { + ModuleKind::Mod => { + let module_id = module.def_id; + visible_module_children(self.genv.tcx(), module_id, self.current_module.to_def_id()) + .find(|child| child.ident == ident) + .and_then(|child| fhir::Res::try_from(child.res).ok()) + } + ModuleKind::Trait => { + let tcx = self.genv.tcx(); + let trait_id = module.def_id; + tcx.associated_items(trait_id) + .find_by_name_and_namespace(tcx, ident, ns, trait_id) + .map(|assoc| fhir::Res::Def(assoc.kind.as_def_kind(), assoc.def_id)) + } + } } pub fn into_output(self) -> Result { @@ -411,7 +436,7 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { self.define_generics(def_id); self.define_res_in( kw::SelfUpper, - hir::def::Res::SelfTyParam { trait_: def_id.resolved_id() }, + fhir::Res::SelfTyParam { trait_: def_id.resolved_id() }, TypeNS, ); self.resolve_trait(def_id).collect_err(&mut self.err); @@ -420,9 +445,8 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { self.define_generics(def_id); self.define_res_in( kw::SelfUpper, - hir::def::Res::SelfTyAlias { + fhir::Res::SelfTyAlias { alias_to: def_id.resolved_id(), - forbid_generic: false, is_trait_impl: impl_.of_trait.is_some(), }, TypeNS, @@ -437,11 +461,7 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { self.define_generics(def_id); self.define_res_in( kw::SelfUpper, - hir::def::Res::SelfTyAlias { - alias_to: def_id.resolved_id(), - forbid_generic: false, - is_trait_impl: false, - }, + fhir::Res::SelfTyAlias { alias_to: def_id.resolved_id(), is_trait_impl: false }, TypeNS, ); self.resolve_enum_def(def_id).collect_err(&mut self.err); @@ -450,11 +470,7 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { self.define_generics(def_id); self.define_res_in( kw::SelfUpper, - hir::def::Res::SelfTyAlias { - alias_to: def_id.resolved_id(), - forbid_generic: false, - is_trait_impl: false, - }, + fhir::Res::SelfTyAlias { alias_to: def_id.resolved_id(), is_trait_impl: false }, TypeNS, ); self.resolve_struct_def(def_id).collect_err(&mut self.err); @@ -506,6 +522,24 @@ impl<'tcx> hir::intravisit::Visitor<'tcx> for CrateResolver<'_, 'tcx> { } } +/// Akin to [`rustc_resolve::Module`] but specialized to what we support +struct Module { + kind: ModuleKind, + def_id: DefId, +} + +impl Module { + fn new(kind: ModuleKind, def_id: DefId) -> Self { + Self { kind, def_id } + } +} + +/// Akin to [`rustc_resolve::ModuleKind`] but specialized to what we support +enum ModuleKind { + Mod, + Trait, +} + #[derive(Debug)] enum RibKind { /// Any other rib without extra rules. @@ -517,7 +551,7 @@ enum RibKind { #[derive(Debug)] struct Rib { kind: RibKind, - bindings: FxHashMap, + bindings: FxHashMap, } impl Rib { @@ -555,7 +589,7 @@ fn is_prelude_import(tcx: TyCtxt, item: &hir::Item) -> bool { } /// Abstraction over [`surface::PathSegment`] and [`surface::ExprPathSegment`] -trait Segment { +trait Segment: std::fmt::Debug { fn record_segment_res(resolver: &mut CrateResolver, segment: &Self, res: fhir::Res); fn ident(&self) -> Ident; } @@ -675,21 +709,6 @@ impl surface::visit::Visitor for ItemResolver<'_, '_, '_> { } } -fn map_res(res: hir::def::Res) -> hir::def::Res { - match res { - hir::def::Res::Def(k, id) => hir::def::Res::Def(k, id), - hir::def::Res::PrimTy(pty) => hir::def::Res::PrimTy(pty), - hir::def::Res::SelfTyParam { trait_ } => hir::def::Res::SelfTyParam { trait_ }, - hir::def::Res::SelfTyAlias { alias_to, forbid_generic, is_trait_impl } => { - hir::def::Res::SelfTyAlias { alias_to, forbid_generic, is_trait_impl } - } - hir::def::Res::SelfCtor(id) => hir::def::Res::SelfCtor(id), - hir::def::Res::ToolMod => hir::def::Res::ToolMod, - hir::def::Res::NonMacroAttr(nma) => hir::def::Res::NonMacroAttr(nma), - hir::def::Res::Err => hir::def::Res::Err, - } -} - struct OpaqueTypeCollector<'sess> { opaque: Option, // TODO: HACK! need to generalize to multiple opaque types/impls in a signature. errors: Errors<'sess>, @@ -751,7 +770,7 @@ fn builtin_types_rib() -> Rib { kind: RibKind::Normal, bindings: PrimTy::ALL .into_iter() - .map(|pty| (pty.name(), hir::def::Res::PrimTy(pty))) + .map(|pty| (pty.name(), fhir::Res::PrimTy(pty))) .collect(), } } diff --git a/crates/flux-fhir-analysis/locales/en-US.ftl b/crates/flux-fhir-analysis/locales/en-US.ftl index 7f506f1bc7..7041b2841c 100644 --- a/crates/flux-fhir-analysis/locales/en-US.ftl +++ b/crates/flux-fhir-analysis/locales/en-US.ftl @@ -163,6 +163,7 @@ fhir_analysis_assoc_type_not_found = fhir_analysis_ambiguous_assoc_type = ambiguous associated type `{$name}` + .label = help: use fully-qualified syntax fhir_analysis_invalid_base_instance = values of this type cannot be used as base sorted instances @@ -268,3 +269,6 @@ fhir_analysis_refine_arg_mismatch = [one] argument *[other] arguments } + +fhir_analysis_expected_type = + expected a type, found {$def_descr} `{$name}` diff --git a/crates/flux-fhir-analysis/src/conv/mod.rs b/crates/flux-fhir-analysis/src/conv/mod.rs index cd73d53f1a..dac443afc0 100644 --- a/crates/flux-fhir-analysis/src/conv/mod.rs +++ b/crates/flux-fhir-analysis/src/conv/mod.rs @@ -1405,22 +1405,22 @@ impl<'genv, 'tcx: 'genv, P: ConvPhase<'genv, 'tcx>> ConvCtxt

{ fn probe_single_bound_for_assoc_item( &self, all_candidates: impl Fn() -> I, - assoc_ident: rustc_span::symbol::Ident, - ) -> Result, ErrorGuaranteed> + assoc_name: rustc_span::symbol::Ident, + ) -> QueryResult> where I: Iterator>, { let mut matching_candidates = all_candidates().filter(|r| { - self.trait_defines_associated_item_named(r.def_id(), AssocKind::Type, assoc_ident) + self.trait_defines_associated_item_named(r.def_id(), AssocKind::Type, assoc_name) .is_some() }); let Some(bound) = matching_candidates.next() else { - return Err(self.emit(errors::AssocTypeNotFound::new(assoc_ident))); + return Err(self.emit(errors::AssocTypeNotFound::new(assoc_name)))?; }; if matching_candidates.next().is_some() { - return Err(self.emit(errors::AmbiguousAssocType::new(assoc_ident))); + self.report_ambiguous_assoc_ty(assoc_name.span, assoc_name)?; } Ok(bound) @@ -1539,16 +1539,18 @@ impl<'genv, 'tcx: 'genv, P: ConvPhase<'genv, 'tcx>> ConvCtxt

{ } fhir::Res::Def(DefKind::AssocTy, assoc_id) => { let trait_id = self.tcx().trait_of_item(assoc_id).unwrap(); + let [.., trait_segment, assoc_segment] = path.segments else { span_bug!(path.span, "expected at least two segments"); }; + let Some(qself) = qself else { + self.report_ambiguous_assoc_ty(path.span, assoc_segment.ident)? + }; + let trait_generics = self.genv().generics_of(trait_id)?; - let qself = self.conv_ty_to_generic_arg( - env, - &trait_generics.own_params[0], - qself.unwrap(), - )?; + let qself = + self.conv_ty_to_generic_arg(env, &trait_generics.own_params[0], qself)?; let mut args = vec![qself]; self.conv_generic_args_into(env, trait_id, trait_segment, &mut args)?; self.conv_generic_args_into(env, assoc_id, assoc_segment, &mut args)?; @@ -1581,7 +1583,8 @@ impl<'genv, 'tcx: 'genv, P: ConvPhase<'genv, 'tcx>> ConvCtxt

{ ) } } - fhir::Res::Def(..) | fhir::Res::Err => { + fhir::Res::Def(kind, def_id) => self.report_expected_type(path.span, kind, def_id)?, + fhir::Res::Err => { span_bug!(path.span, "unexpected resolution in conv_ty_ctor: {:?}", path.res) } }; @@ -1756,6 +1759,27 @@ impl<'genv, 'tcx: 'genv, P: ConvPhase<'genv, 'tcx>> ConvCtxt

{ fn emit(&self, err: impl Diagnostic<'genv>) -> ErrorGuaranteed { self.genv().sess().emit_err(err) } + + fn report_ambiguous_assoc_ty( + &self, + span: Span, + assoc_name: Ident, + ) -> Result { + Err(self.emit(errors::AmbiguousAssocType { span, name: assoc_name }))? + } + + fn report_expected_type( + &self, + span: Span, + kind: DefKind, + def_id: DefId, + ) -> Result { + Err(self.emit(errors::ExpectedType { + span, + def_descr: self.tcx().def_kind_descr(kind, def_id), + name: self.tcx().def_path_str(def_id), + }))? + } } /// Check generic params for types @@ -2321,14 +2345,9 @@ mod errors { #[diag(fhir_analysis_ambiguous_assoc_type, code = E0999)] pub(super) struct AmbiguousAssocType { #[primary_span] - span: Span, - name: Ident, - } - - impl AmbiguousAssocType { - pub(super) fn new(assoc_ident: Ident) -> Self { - Self { span: assoc_ident.span, name: assoc_ident } - } + #[label] + pub span: Span, + pub name: Ident, } #[derive(Diagnostic)] @@ -2571,4 +2590,13 @@ mod errors { pub found: usize, pub kind: &'static str, } + + #[derive(Diagnostic)] + #[diag(fhir_analysis_expected_type, code = E0999)] + pub(super) struct ExpectedType { + #[primary_span] + pub span: Span, + pub def_descr: &'static str, + pub name: String, + } } diff --git a/crates/flux-fhir-analysis/src/lib.rs b/crates/flux-fhir-analysis/src/lib.rs index 6d02d975b8..4ea282ceac 100644 --- a/crates/flux-fhir-analysis/src/lib.rs +++ b/crates/flux-fhir-analysis/src/lib.rs @@ -1,4 +1,4 @@ -#![feature(rustc_private, let_chains, box_patterns, if_let_guard, once_cell_try)] +#![feature(rustc_private, let_chains, box_patterns, if_let_guard, once_cell_try, never_type)] extern crate rustc_ast; extern crate rustc_data_structures; diff --git a/tests/tests/neg/error_messages/conv/ambiguous_assoc_ty.rs b/tests/tests/neg/error_messages/conv/ambiguous_assoc_ty.rs new file mode 100644 index 0000000000..8a5b439f46 --- /dev/null +++ b/tests/tests/neg/error_messages/conv/ambiguous_assoc_ty.rs @@ -0,0 +1,10 @@ +extern crate flux_rs; + +use flux_rs::*; + +pub trait MyTrait { + type A; + + #[sig(fn(_) -> MyTrait::A)] //~ ERROR ambiguous associated type + fn f(&self) -> ::A; +} diff --git a/tests/tests/neg/error_messages/conv/expected_type.rs b/tests/tests/neg/error_messages/conv/expected_type.rs new file mode 100644 index 0000000000..7dcedac3e6 --- /dev/null +++ b/tests/tests/neg/error_messages/conv/expected_type.rs @@ -0,0 +1,10 @@ +trait A {} + +#[flux_rs::sig(fn(x: A))] //~ ERROR expected a type, found trait +fn test00(x: i32) {} + +#[allow(non_snake_case)] +mod B {} + +#[flux_rs::sig(fn(x: B))] //~ ERROR expected a type, found module +fn test01(x: i32) {}