From e9c3b409bdb188b9ba261d3cd6608723c77f4c46 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Mon, 30 Oct 2023 15:56:49 -0700 Subject: [PATCH 1/7] Support Unimplemented attribute This change introduces the @Unimplemented() attributed which can be used to mark an item as something that should not be used. The motivation behind including this and not just treating it as a Not Found error is to give the user more information that this API may have previously existing and is not currently supported (see #699 for context). --- .../qsc_frontend/src/compile/preprocess.rs | 3 +- compiler/qsc_frontend/src/compile/tests.rs | 178 +++++++++++++++++- compiler/qsc_frontend/src/lower.rs | 60 +++--- compiler/qsc_frontend/src/resolve.rs | 70 ++++++- compiler/qsc_frontend/src/typeck/check.rs | 3 +- compiler/qsc_frontend/src/typeck/tests.rs | 10 +- compiler/qsc_hir/src/global.rs | 3 +- compiler/qsc_hir/src/hir.rs | 42 +++++ compiler/qsc_passes/src/entry_point.rs | 5 +- language_service/src/qsc_utils.rs | 3 +- 10 files changed, 329 insertions(+), 48 deletions(-) diff --git a/compiler/qsc_frontend/src/compile/preprocess.rs b/compiler/qsc_frontend/src/compile/preprocess.rs index 7d939816e2..cfbf5bd617 100644 --- a/compiler/qsc_frontend/src/compile/preprocess.rs +++ b/compiler/qsc_frontend/src/compile/preprocess.rs @@ -6,6 +6,7 @@ use qsc_ast::{ ast::{Attr, ExprKind, ItemKind, Namespace, Stmt, StmtKind}, mut_visit::MutVisitor, }; +use qsc_hir::hir; use std::rc::Rc; use super::TargetProfile; @@ -119,7 +120,7 @@ impl MutVisitor for Conditional { fn matches_target(attrs: &[Box], target: TargetProfile) -> bool { attrs.iter().all(|attr| { - if attr.name.name.as_ref() == "Config" { + if hir::Attr::from_str(attr.name.name.as_ref()) == Ok(hir::Attr::Config) { if let ExprKind::Paren(inner) = attr.arg.kind.as_ref() { match inner.kind.as_ref() { ExprKind::Path(path) => { diff --git a/compiler/qsc_frontend/src/compile/tests.rs b/compiler/qsc_frontend/src/compile/tests.rs index d172ec2810..aec5c1a8ae 100644 --- a/compiler/qsc_frontend/src/compile/tests.rs +++ b/compiler/qsc_frontend/src/compile/tests.rs @@ -11,8 +11,8 @@ use qsc_data_structures::span::Span; use qsc_hir::{ global, hir::{ - Block, Expr, ExprKind, ItemId, ItemKind, Lit, LocalItemId, NodeId, Res, SpecBody, Stmt, - StmtKind, + Block, Expr, ExprKind, ItemId, ItemKind, ItemStatus, Lit, LocalItemId, NodeId, Res, + SpecBody, Stmt, StmtKind, }, mut_visit::MutVisitor, ty::{Prim, Ty}, @@ -268,6 +268,7 @@ fn entry_call_operation() { &Res::Item(ItemId { package: None, item: LocalItemId::from(1), + status: ItemStatus::Normal, }), res ); @@ -876,3 +877,176 @@ fn two_files_error_eof() { Namespace (Ident 1 [26-29] "Bar"): "#]] .assert_eq(&unit.package.to_string()); } + +#[test] +fn unimplemented_call_from_dependency_produces_error() { + let lib_sources = SourceMap::new( + [( + "lib".into(), + indoc! {" + namespace Foo { + @Unimplemented() + operation Bar() : Unit {} + } + "} + .into(), + )], + None, + ); + let mut store = PackageStore::new(super::core()); + let lib = compile(&store, &[], lib_sources, TargetProfile::Full); + assert!(lib.errors.is_empty(), "{:#?}", lib.errors); + let lib = store.insert(lib); + + let sources = SourceMap::new( + [( + "test".into(), + indoc! {" + namespace Test { + open Foo; + operation Main() : Unit { + Bar(); + } + } + "} + .into(), + )], + None, + ); + let unit = compile(&store, &[lib], sources, TargetProfile::Full); + expect![[r#" + [ + Error( + Resolve( + Unimplemented( + "Bar", + Span { + lo: 69, + hi: 72, + }, + ), + ), + ), + ] + "#]] + .assert_debug_eq(&unit.errors); +} + +#[test] +fn unimplemented_attribute_call_within_unit_error() { + let sources = SourceMap::new( + [( + "test".into(), + indoc! {" + namespace Foo { + @Unimplemented() + operation Bar() : Unit {} + operation Baz() : Unit { + Bar(); + } + } + "} + .into(), + )], + None, + ); + let store = PackageStore::new(super::core()); + let unit = compile(&store, &[], sources, TargetProfile::Full); + expect![[r#" + [ + Error( + Resolve( + Unimplemented( + "Bar", + Span { + lo: 104, + hi: 107, + }, + ), + ), + ), + ] + "#]] + .assert_debug_eq(&unit.errors); +} + +#[test] +fn unimplemented_attribute_with_non_unit_expr_error() { + let sources = SourceMap::new( + [( + "test".into(), + indoc! {" + namespace Foo { + @Unimplemented(1) + operation Bar() : Unit {} + } + "} + .into(), + )], + None, + ); + let store = PackageStore::new(super::core()); + let unit = compile(&store, &[], sources, TargetProfile::Full); + expect![[r#" + [ + Error( + Lower( + InvalidAttrArgs( + "()", + Span { + lo: 34, + hi: 37, + }, + ), + ), + ), + ] + "#]] + .assert_debug_eq(&unit.errors); +} + +#[test] +fn unimplemented_attribute_avoids_ambiguous_error_with_duplicate_names_in_scope() { + let lib_sources = SourceMap::new( + [( + "lib".into(), + indoc! {" + namespace Foo { + @Unimplemented() + operation Bar() : Unit {} + } + "} + .into(), + )], + None, + ); + let mut store = PackageStore::new(super::core()); + let lib = compile(&store, &[], lib_sources, TargetProfile::Full); + assert!(lib.errors.is_empty(), "{:#?}", lib.errors); + let lib = store.insert(lib); + + let sources = SourceMap::new( + [( + "test".into(), + indoc! {" + namespace Dependency { + operation Bar() : Unit {} + } + namespace Test { + open Foo; + open Dependency; + operation Main() : Unit { + Bar(); + } + } + "} + .into(), + )], + None, + ); + let unit = compile(&store, &[lib], sources, TargetProfile::Full); + expect![[r#" + [] + "#]] + .assert_debug_eq(&unit.errors); +} diff --git a/compiler/qsc_frontend/src/lower.rs b/compiler/qsc_frontend/src/lower.rs index 9664278f1d..be5fa1dbcf 100644 --- a/compiler/qsc_frontend/src/lower.rs +++ b/compiler/qsc_frontend/src/lower.rs @@ -19,7 +19,7 @@ use qsc_hir::{ mut_visit::MutVisitor, ty::{Arrow, FunctorSetValue, Ty}, }; -use std::{clone::Clone, rc::Rc, vec}; +use std::{clone::Clone, rc::Rc, str::FromStr, vec}; use thiserror::Error; #[derive(Clone, Debug, Diagnostic, Error)] @@ -167,7 +167,7 @@ impl With<'_> { }; let resolve_id = |id| match self.names.get(id) { - Some(&resolve::Res::Item(hir::ItemId { item, .. })) => item, + Some(&resolve::Res::Item(item)) => item, _ => panic!("item should have item ID"), }; @@ -176,7 +176,7 @@ impl With<'_> { ast::ItemKind::Callable(callable) => { let id = resolve_id(callable.name.id); let grandparent = self.lowerer.parent; - self.lowerer.parent = Some(id); + self.lowerer.parent = Some(id.item); let callable = self.lower_callable_decl(callable); self.lowerer.parent = grandparent; (id, hir::ItemKind::Callable(callable)) @@ -186,10 +186,7 @@ impl With<'_> { let udt = self .tys .udts - .get(&hir::ItemId { - package: None, - item: id, - }) + .get(&id) .expect("type item should have lowered UDT"); (id, hir::ItemKind::Ty(self.lower_ident(name), udt.clone())) @@ -197,7 +194,7 @@ impl With<'_> { }; self.lowerer.items.push(hir::Item { - id, + id: id.item, span: item.span, parent: self.lowerer.parent, doc: Rc::clone(&item.doc), @@ -206,12 +203,12 @@ impl With<'_> { kind, }); - Some(id) + Some(id.item) } fn lower_attr(&mut self, attr: &ast::Attr) -> Option { - if attr.name.name.as_ref() == "EntryPoint" { - match &*attr.arg.kind { + match hir::Attr::from_str(attr.name.name.as_ref()) { + Ok(hir::Attr::EntryPoint) => match &*attr.arg.kind { ast::ExprKind::Tuple(args) if args.is_empty() => Some(hir::Attr::EntryPoint), _ => { self.lowerer @@ -219,23 +216,34 @@ impl With<'_> { .push(Error::InvalidAttrArgs("()", attr.arg.span)); None } + }, + Ok(hir::Attr::Unimplemented) => match &*attr.arg.kind { + ast::ExprKind::Tuple(args) if args.is_empty() => Some(hir::Attr::Unimplemented), + _ => { + self.lowerer + .errors + .push(Error::InvalidAttrArgs("()", attr.arg.span)); + None + } + }, + Ok(hir::Attr::Config) => { + if !matches!(attr.arg.kind.as_ref(), ast::ExprKind::Paren(inner) + if matches!(inner.kind.as_ref(), ast::ExprKind::Path(path) + if TargetProfile::from_str(path.name.name.as_ref()).is_ok())) + { + self.lowerer + .errors + .push(Error::InvalidAttrArgs("Full or Base", attr.arg.span)); + } + None } - } else if attr.name.name.as_ref() == "Config" { - if !matches!(attr.arg.kind.as_ref(), ast::ExprKind::Paren(inner) - if matches!(inner.kind.as_ref(), ast::ExprKind::Path(path) - if TargetProfile::is_target_str(path.name.name.as_ref()))) - { - self.lowerer - .errors - .push(Error::InvalidAttrArgs("Full or Base", attr.arg.span)); + Err(()) => { + self.lowerer.errors.push(Error::UnknownAttr( + attr.name.name.to_string(), + attr.name.span, + )); + None } - None - } else { - self.lowerer.errors.push(Error::UnknownAttr( - attr.name.name.to_string(), - attr.name.span, - )); - None } } diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index a2ef7b81cd..2c206b9a13 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -7,17 +7,17 @@ mod tests; use miette::Diagnostic; use qsc_ast::{ ast::{self, CallableDecl, Ident, NodeId, TopLevelNode}, - visit::{self as ast_visit, Visitor as AstVisitor}, + visit::{self as ast_visit, walk_attr, Visitor as AstVisitor}, }; use qsc_data_structures::{index_map::IndexMap, span::Span}; use qsc_hir::{ assigner::Assigner, global, - hir::{self, ItemId, LocalItemId, PackageId}, + hir::{self, ItemId, ItemStatus, LocalItemId, PackageId}, ty::{ParamId, Prim}, }; use rustc_hash::{FxHashMap, FxHashSet}; -use std::{collections::hash_map::Entry, rc::Rc, vec}; +use std::{collections::hash_map::Entry, rc::Rc, str::FromStr, vec}; use thiserror::Error; use crate::compile::preprocess::TrackedName; @@ -93,6 +93,11 @@ pub(super) enum Error { ))] #[diagnostic(code("Qsc.Resolve.NotFound"))] NotAvailable(String, String, #[label] Span), + + #[error("use of unimplemented item `{0}`")] + #[diagnostic(help("this item is not implemented and cannot be used"))] + #[diagnostic(code("Qsc.Resolve.Unimplemented"))] + Unimplemented(String, #[label] Span), } struct Scope { @@ -238,10 +243,22 @@ impl Resolver { } } + fn check_item_status(&mut self, res: Res, name: String, span: Span) { + match res { + Res::Item(item) if item.status == ItemStatus::Unimplemented => { + self.errors.push(Error::Unimplemented(name, span)); + } + _ => {} + } + } + fn resolve_ident(&mut self, kind: NameKind, name: &Ident) { let namespace = None; match resolve(kind, &self.globals, &self.scopes, name, &namespace) { - Ok(id) => self.names.insert(name.id, id), + Ok(res) => { + self.check_item_status(res, name.name.to_string(), name.span); + self.names.insert(name.id, res); + } Err(err) => self.errors.push(err), } } @@ -250,7 +267,10 @@ impl Resolver { let name = &path.name; let namespace = &path.namespace; match resolve(kind, &self.globals, &self.scopes, name, namespace) { - Ok(id) => self.names.insert(path.id, id), + Ok(res) => { + self.check_item_status(res, path.name.name.to_string(), path.span); + self.names.insert(path.id, res); + } Err(err) => { if let Error::NotFound(name, span) = err { if let Some(dropped_name) = @@ -390,8 +410,13 @@ impl AstVisitor<'_> for With<'_> { }); } - // We do not perform name resolution on attributes, as those are checking during lowering. - fn visit_attr(&mut self, _: &ast::Attr) {} + fn visit_attr(&mut self, attr: &ast::Attr) { + if hir::Attr::from_str(attr.name.name.as_ref()) == Ok(hir::Attr::Config) { + // The Config attribute arguments do not go through name resolution. + return; + } + walk_attr(self, attr); + } fn visit_callable_decl(&mut self, decl: &ast::CallableDecl) { fn collect_param_names(pat: &ast::Pat, names: &mut FxHashSet>) { @@ -645,6 +670,13 @@ fn is_field_update(globals: &GlobalScope, scopes: &[Scope], index: &ast::Expr) - } } +fn ast_attrs_as_hir_attrs(attrs: &[Box]) -> Vec { + attrs + .iter() + .filter_map(|attr| hir::Attr::from_str(attr.name.name.as_ref()).ok()) + .collect() +} + fn bind_global_item( names: &mut Names, scope: &mut GlobalScope, @@ -654,7 +686,9 @@ fn bind_global_item( ) -> Result<(), Error> { match &*item.kind { ast::ItemKind::Callable(decl) => { - let res = Res::Item(next_id()); + let mut item_id = next_id(); + item_id.status = ItemStatus::from_attrs(&ast_attrs_as_hir_attrs(item.attrs.as_ref())); + let res = Res::Item(item_id); names.insert(decl.name.id, res); match scope .terms @@ -674,7 +708,9 @@ fn bind_global_item( } } ast::ItemKind::Ty(name, _) => { - let res = Res::Item(next_id()); + let mut item_id = next_id(); + item_id.status = ItemStatus::from_attrs(&ast_attrs_as_hir_attrs(item.attrs.as_ref())); + let res = Res::Item(item_id); names.insert(name.id, res); match ( scope @@ -771,6 +807,21 @@ fn resolve( } } + if candidates.len() > 1 { + // If there are multiple candidates, remove unimplemented items. + let mut removals = Vec::new(); + for res in candidates.keys() { + if let Res::Item(item) = res { + if item.status == ItemStatus::Unimplemented { + removals.push(*res); + } + } + } + for res in removals { + candidates.remove(&res); + } + } + if candidates.len() > 1 { let mut opens: Vec<_> = candidates.into_values().collect(); opens.sort_unstable_by_key(|open| open.span); @@ -867,6 +918,7 @@ fn intrapackage(item: LocalItemId) -> ItemId { ItemId { package: None, item, + status: ItemStatus::Normal, } } diff --git a/compiler/qsc_frontend/src/typeck/check.rs b/compiler/qsc_frontend/src/typeck/check.rs index 118fc5de55..2346b4f5d0 100644 --- a/compiler/qsc_frontend/src/typeck/check.rs +++ b/compiler/qsc_frontend/src/typeck/check.rs @@ -16,7 +16,7 @@ use qsc_ast::{ }; use qsc_data_structures::index_map::IndexMap; use qsc_hir::{ - hir::{self, ItemId, PackageId}, + hir::{self, ItemId, ItemStatus, PackageId}, ty::{FunctorSetValue, Scheme, Ty, Udt}, }; use rustc_hash::FxHashMap; @@ -42,6 +42,7 @@ impl GlobalTable { let item_id = ItemId { package: Some(id), item: item.id, + status: ItemStatus::from_attrs(&item.attrs), }; match &item.kind { diff --git a/compiler/qsc_frontend/src/typeck/tests.rs b/compiler/qsc_frontend/src/typeck/tests.rs index 01f17134a1..e1fd057c43 100644 --- a/compiler/qsc_frontend/src/typeck/tests.rs +++ b/compiler/qsc_frontend/src/typeck/tests.rs @@ -1125,7 +1125,7 @@ fn ternop_update_udt_unknown_field_name() { #36 133-134 "p" : UDT #39 138-143 "Third" : ? #42 147-148 "3" : Int - Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1) })), "Third", Span { lo: 133, hi: 148 })))) + Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), "Third", Span { lo: 133, hi: 148 })))) "#]], ); } @@ -1162,7 +1162,7 @@ fn ternop_update_udt_unknown_field_name_known_global() { #42 163-164 "p" : UDT #45 168-173 "Third" : ? #48 177-178 "3" : Int - Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1) })), "Third", Span { lo: 163, hi: 178 })))) + Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), "Third", Span { lo: 163, hi: 178 })))) "#]], ); } @@ -2018,7 +2018,7 @@ fn newtype_does_not_match_base_ty() { #19 67-73 "NewInt" : (Int -> UDT) #22 73-76 "(5)" : Int #23 74-75 "5" : Int - Error(Type(Error(TyMismatch(Prim(Int), Udt(Item(ItemId { package: None, item: LocalItemId(1) })), Span { lo: 67, hi: 76 })))) + Error(Type(Error(TyMismatch(Prim(Int), Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), Span { lo: 67, hi: 76 })))) "#]], ); } @@ -2041,7 +2041,7 @@ fn newtype_does_not_match_other_newtype() { #25 99-106 "NewInt1" : (Int -> UDT) #28 106-109 "(5)" : Int #29 107-108 "5" : Int - Error(Type(Error(TyMismatch(Udt(Item(ItemId { package: None, item: LocalItemId(2) })), Udt(Item(ItemId { package: None, item: LocalItemId(1) })), Span { lo: 99, hi: 109 })))) + Error(Type(Error(TyMismatch(Udt(Item(ItemId { package: None, item: LocalItemId(2), status: Normal })), Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), Span { lo: 99, hi: 109 })))) "#]], ); } @@ -2111,7 +2111,7 @@ fn newtype_field_invalid() { #22 88-89 "y" : ?1 #24 92-99 "x::Nope" : ?1 #25 92-93 "x" : UDT - Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1) })), "Nope", Span { lo: 92, hi: 99 })))) + Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), "Nope", Span { lo: 92, hi: 99 })))) Error(Type(Error(AmbiguousTy(Span { lo: 92, hi: 99 })))) "#]], ); diff --git a/compiler/qsc_hir/src/global.rs b/compiler/qsc_hir/src/global.rs index 9d2b7feaf9..67cbffa386 100644 --- a/compiler/qsc_hir/src/global.rs +++ b/compiler/qsc_hir/src/global.rs @@ -2,7 +2,7 @@ // Licensed under the MIT License. use crate::{ - hir::{Item, ItemId, ItemKind, Package, PackageId, Visibility}, + hir::{Item, ItemId, ItemKind, ItemStatus, Package, PackageId, Visibility}, ty::Scheme, }; use qsc_data_structures::index_map; @@ -94,6 +94,7 @@ impl PackageIter<'_> { let id = ItemId { package: self.id, item: item.id, + status: ItemStatus::from_attrs(item.attrs.as_ref()), }; match (&item.kind, &parent) { diff --git a/compiler/qsc_hir/src/hir.rs b/compiler/qsc_hir/src/hir.rs index dc0c1a5dc1..07c4e8e0ec 100644 --- a/compiler/qsc_hir/src/hir.rs +++ b/compiler/qsc_hir/src/hir.rs @@ -174,6 +174,8 @@ pub struct ItemId { pub package: Option, /// The item ID. pub item: LocalItemId, + /// The item status. + pub status: ItemStatus, } impl Display for ItemId { @@ -185,6 +187,28 @@ impl Display for ItemId { } } +/// The status of an item. +#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub enum ItemStatus { + /// The item is defined normally. + Normal, + /// The item is marked as unimplemented and uses are disallowed. + Unimplemented, +} + +impl ItemStatus { + /// Create an item status from the given attributes list. + #[must_use] + pub fn from_attrs(attrs: &[Attr]) -> Self { + for attr in attrs { + if *attr == Attr::Unimplemented { + return Self::Unimplemented; + } + } + Self::Normal + } +} + /// A resolution. This connects a usage of a name with the declaration of that name by uniquely /// identifying the node that declared it. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] @@ -205,6 +229,7 @@ impl Res { Res::Item(id) if id.package.is_none() => Res::Item(ItemId { package: Some(package), item: id.item, + status: id.status, }), _ => *self, } @@ -1127,8 +1152,25 @@ impl Display for Ident { /// An attribute. #[derive(Clone, Debug, PartialEq)] pub enum Attr { + /// Provide pre-processing information about when an item should be included in compilation. + Config, /// Indicates that a callable is an entry point to a program. EntryPoint, + /// Indicates that an item does not have an implementation available for use. + Unimplemented, +} + +impl FromStr for Attr { + type Err = (); + + fn from_str(s: &str) -> result::Result { + match s { + "Config" => Ok(Self::Config), + "EntryPoint" => Ok(Self::EntryPoint), + "Unimplemented" => Ok(Self::Unimplemented), + _ => Err(()), + } + } } /// A field. diff --git a/compiler/qsc_passes/src/entry_point.rs b/compiler/qsc_passes/src/entry_point.rs index 784964a52f..c1d7234121 100644 --- a/compiler/qsc_passes/src/entry_point.rs +++ b/compiler/qsc_passes/src/entry_point.rs @@ -10,8 +10,8 @@ use qsc_data_structures::span::Span; use qsc_hir::{ assigner::Assigner, hir::{ - Attr, CallableDecl, Expr, ExprKind, Item, ItemId, ItemKind, LocalItemId, Package, PatKind, - Res, + Attr, CallableDecl, Expr, ExprKind, Item, ItemId, ItemKind, ItemStatus, LocalItemId, + Package, PatKind, Res, }, ty::Ty, visit::Visitor, @@ -89,6 +89,7 @@ fn create_entry_from_callables( let item_id = ItemId { package: None, item, + status: ItemStatus::Normal, }; let callee = Expr { id: assigner.next_node(), diff --git a/language_service/src/qsc_utils.rs b/language_service/src/qsc_utils.rs index 0c531250f8..9b41614d2b 100644 --- a/language_service/src/qsc_utils.rs +++ b/language_service/src/qsc_utils.rs @@ -4,7 +4,7 @@ use qsc::{ ast, compile::{self, Error}, - hir::{self, Item, ItemId, Package, PackageId}, + hir::{self, Item, ItemId, ItemStatus, Package, PackageId}, CompileUnit, PackageStore, PackageType, SourceMap, Span, TargetProfile, }; @@ -180,6 +180,7 @@ fn resolve_item<'a>( ItemId { package: package_id, item: item_id.item, + status: ItemStatus::Normal, }, ) } From ad41766bef3cb6cb7eafd4737d366659abc3821d Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Tue, 31 Oct 2023 10:04:14 -0700 Subject: [PATCH 2/7] Adding comment for duplicate pruning --- compiler/qsc_frontend/src/resolve.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index 2c206b9a13..f3db05413c 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -808,7 +808,9 @@ fn resolve( } if candidates.len() > 1 { - // If there are multiple candidates, remove unimplemented items. + // If there are multiple candidates, remove unimplemented items. This allows resolution to + // succeed in cases where both an older, unimplimented API and newer, implemented API with the + // same name are both in scope without forcing the user to fully qualify the name. let mut removals = Vec::new(); for res in candidates.keys() { if let Res::Item(item) = res { From 97062fd8be5e3bb9134d2da71f4266c64f34cc6b Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Tue, 31 Oct 2023 11:07:20 -0700 Subject: [PATCH 3/7] Move `ItemStatus` into resolver `Res` instead of `ItemId` --- compiler/qsc_frontend/src/compile/tests.rs | 5 +- compiler/qsc_frontend/src/lower.rs | 6 +-- compiler/qsc_frontend/src/resolve.rs | 52 ++++++++++++--------- compiler/qsc_frontend/src/resolve/tests.rs | 2 +- compiler/qsc_frontend/src/typeck/check.rs | 7 ++- compiler/qsc_frontend/src/typeck/convert.rs | 2 +- compiler/qsc_frontend/src/typeck/rules.rs | 4 +- compiler/qsc_frontend/src/typeck/tests.rs | 30 ++++++------ compiler/qsc_hir/src/global.rs | 7 ++- compiler/qsc_hir/src/hir.rs | 3 -- compiler/qsc_passes/src/entry_point.rs | 5 +- language_service/src/name_locator.rs | 4 +- language_service/src/qsc_utils.rs | 3 +- language_service/src/rename.rs | 5 +- language_service/src/signature_help.rs | 2 +- 15 files changed, 71 insertions(+), 66 deletions(-) diff --git a/compiler/qsc_frontend/src/compile/tests.rs b/compiler/qsc_frontend/src/compile/tests.rs index aec5c1a8ae..51f03e55dc 100644 --- a/compiler/qsc_frontend/src/compile/tests.rs +++ b/compiler/qsc_frontend/src/compile/tests.rs @@ -11,8 +11,8 @@ use qsc_data_structures::span::Span; use qsc_hir::{ global, hir::{ - Block, Expr, ExprKind, ItemId, ItemKind, ItemStatus, Lit, LocalItemId, NodeId, Res, - SpecBody, Stmt, StmtKind, + Block, Expr, ExprKind, ItemId, ItemKind, Lit, LocalItemId, NodeId, Res, SpecBody, Stmt, + StmtKind, }, mut_visit::MutVisitor, ty::{Prim, Ty}, @@ -268,7 +268,6 @@ fn entry_call_operation() { &Res::Item(ItemId { package: None, item: LocalItemId::from(1), - status: ItemStatus::Normal, }), res ); diff --git a/compiler/qsc_frontend/src/lower.rs b/compiler/qsc_frontend/src/lower.rs index be5fa1dbcf..d65fe48231 100644 --- a/compiler/qsc_frontend/src/lower.rs +++ b/compiler/qsc_frontend/src/lower.rs @@ -124,7 +124,7 @@ impl With<'_> { } pub(super) fn lower_namespace(&mut self, namespace: &ast::Namespace) { - let Some(&resolve::Res::Item(hir::ItemId { item: id, .. })) = + let Some(&resolve::Res::Item(hir::ItemId { item: id, .. }, _)) = self.names.get(namespace.name.id) else { panic!("namespace should have item ID"); @@ -167,7 +167,7 @@ impl With<'_> { }; let resolve_id = |id| match self.names.get(id) { - Some(&resolve::Res::Item(item)) => item, + Some(&resolve::Res::Item(item, _)) => item, _ => panic!("item should have item ID"), }; @@ -714,7 +714,7 @@ impl With<'_> { fn lower_path(&mut self, path: &ast::Path) -> hir::Res { match self.names.get(path.id) { - Some(&resolve::Res::Item(item)) => hir::Res::Item(item), + Some(&resolve::Res::Item(item, _)) => hir::Res::Item(item), Some(&resolve::Res::Local(node)) => hir::Res::Local(self.lower_id(node)), Some(resolve::Res::PrimTy(_) | resolve::Res::UnitTy | resolve::Res::Param(_)) | None => hir::Res::Err, diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index f3db05413c..0bd8fb72fb 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -36,7 +36,7 @@ pub(super) type Names = IndexMap; #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub enum Res { /// A global item. - Item(ItemId), + Item(ItemId, ItemStatus), /// A local variable. Local(NodeId), /// A type/functor parameter in the generics section of the parent callable decl. @@ -244,11 +244,8 @@ impl Resolver { } fn check_item_status(&mut self, res: Res, name: String, span: Span) { - match res { - Res::Item(item) if item.status == ItemStatus::Unimplemented => { - self.errors.push(Error::Unimplemented(name, span)); - } - _ => {} + if let Res::Item(_, ItemStatus::Unimplemented) = res { + self.errors.push(Error::Unimplemented(name, span)); } } @@ -334,13 +331,25 @@ impl Resolver { ast::ItemKind::Open(name, alias) => self.bind_open(name, alias), ast::ItemKind::Callable(decl) => { let id = intrapackage(assigner.next_item()); - self.names.insert(decl.name.id, Res::Item(id)); + self.names.insert( + decl.name.id, + Res::Item( + id, + ItemStatus::from_attrs(&ast_attrs_as_hir_attrs(&item.attrs)), + ), + ); let scope = self.scopes.last_mut().expect("binding should have scope"); scope.terms.insert(Rc::clone(&decl.name.name), id); } ast::ItemKind::Ty(name, _) => { let id = intrapackage(assigner.next_item()); - self.names.insert(name.id, Res::Item(id)); + self.names.insert( + name.id, + Res::Item( + id, + ItemStatus::from_attrs(&ast_attrs_as_hir_attrs(&item.attrs)), + ), + ); let scope = self.scopes.last_mut().expect("binding should have scope"); scope.tys.insert(Rc::clone(&name.name), id); scope.terms.insert(Rc::clone(&name.name), id); @@ -595,14 +604,14 @@ impl GlobalTable { .tys .entry(global.namespace) .or_default() - .insert(global.name, Res::Item(ty.id)); + .insert(global.name, Res::Item(ty.id, global.status)); } global::Kind::Term(term) => { self.scope .terms .entry(global.namespace) .or_default() - .insert(global.name, Res::Item(term.id)); + .insert(global.name, Res::Item(term.id, global.status)); } global::Kind::Namespace => { self.scope.namespaces.insert(global.name); @@ -621,7 +630,7 @@ fn bind_global_items( ) { names.insert( namespace.name.id, - Res::Item(intrapackage(assigner.next_item())), + Res::Item(intrapackage(assigner.next_item()), ItemStatus::Normal), ); scope.namespaces.insert(Rc::clone(&namespace.name.name)); @@ -686,9 +695,9 @@ fn bind_global_item( ) -> Result<(), Error> { match &*item.kind { ast::ItemKind::Callable(decl) => { - let mut item_id = next_id(); - item_id.status = ItemStatus::from_attrs(&ast_attrs_as_hir_attrs(item.attrs.as_ref())); - let res = Res::Item(item_id); + let item_id = next_id(); + let status = ItemStatus::from_attrs(&ast_attrs_as_hir_attrs(item.attrs.as_ref())); + let res = Res::Item(item_id, status); names.insert(decl.name.id, res); match scope .terms @@ -708,9 +717,9 @@ fn bind_global_item( } } ast::ItemKind::Ty(name, _) => { - let mut item_id = next_id(); - item_id.status = ItemStatus::from_attrs(&ast_attrs_as_hir_attrs(item.attrs.as_ref())); - let res = Res::Item(item_id); + let item_id = next_id(); + let status = ItemStatus::from_attrs(&ast_attrs_as_hir_attrs(item.attrs.as_ref())); + let res = Res::Item(item_id, status); names.insert(name.id, res); match ( scope @@ -813,10 +822,8 @@ fn resolve( // same name are both in scope without forcing the user to fully qualify the name. let mut removals = Vec::new(); for res in candidates.keys() { - if let Res::Item(item) = res { - if item.status == ItemStatus::Unimplemented { - removals.push(*res); - } + if let Res::Item(_, ItemStatus::Unimplemented) = res { + removals.push(*res); } } for res in removals { @@ -872,7 +879,7 @@ fn resolve_scope_locals( } if let Some(&id) = scope.item(kind, name) { - return Some(Res::Item(id)); + return Some(Res::Item(id, ItemStatus::Normal)); } if let ScopeKind::Namespace(namespace) = &scope.kind { @@ -920,7 +927,6 @@ fn intrapackage(item: LocalItemId) -> ItemId { ItemId { package: None, item, - status: ItemStatus::Normal, } } diff --git a/compiler/qsc_frontend/src/resolve/tests.rs b/compiler/qsc_frontend/src/resolve/tests.rs index 08622744c6..b8deecc13e 100644 --- a/compiler/qsc_frontend/src/resolve/tests.rs +++ b/compiler/qsc_frontend/src/resolve/tests.rs @@ -31,7 +31,7 @@ impl<'a> Renamer<'a> { fn rename(&self, input: &mut String) { for (span, res) in self.changes.iter().rev() { let name = match res { - Res::Item(item) => match item.package { + Res::Item(item, _) => match item.package { None => format!("item{}", item.item), Some(package) => format!("package{package}_item{}", item.item), }, diff --git a/compiler/qsc_frontend/src/typeck/check.rs b/compiler/qsc_frontend/src/typeck/check.rs index 2346b4f5d0..8f10f65c3b 100644 --- a/compiler/qsc_frontend/src/typeck/check.rs +++ b/compiler/qsc_frontend/src/typeck/check.rs @@ -16,7 +16,7 @@ use qsc_ast::{ }; use qsc_data_structures::index_map::IndexMap; use qsc_hir::{ - hir::{self, ItemId, ItemStatus, PackageId}, + hir::{self, ItemId, PackageId}, ty::{FunctorSetValue, Scheme, Ty, Udt}, }; use rustc_hash::FxHashMap; @@ -42,7 +42,6 @@ impl GlobalTable { let item_id = ItemId { package: Some(id), item: item.id, - status: ItemStatus::from_attrs(&item.attrs), }; match &item.kind { @@ -203,7 +202,7 @@ impl Visitor<'_> for ItemCollector<'_> { fn visit_item(&mut self, item: &ast::Item) { match &*item.kind { ast::ItemKind::Callable(decl) => { - let Some(&Res::Item(item)) = self.names.get(decl.name.id) else { + let Some(&Res::Item(item, _)) = self.names.get(decl.name.id) else { panic!("callable should have item ID"); }; @@ -218,7 +217,7 @@ impl Visitor<'_> for ItemCollector<'_> { } ast::ItemKind::Ty(name, def) => { let span = item.span; - let Some(&Res::Item(item)) = self.names.get(name.id) else { + let Some(&Res::Item(item, _)) = self.names.get(name.id) else { panic!("type should have item ID"); }; diff --git a/compiler/qsc_frontend/src/typeck/convert.rs b/compiler/qsc_frontend/src/typeck/convert.rs index 54fcad7cbd..1c16e034e6 100644 --- a/compiler/qsc_frontend/src/typeck/convert.rs +++ b/compiler/qsc_frontend/src/typeck/convert.rs @@ -42,7 +42,7 @@ pub(crate) fn ty_from_ast(names: &Names, ty: &ast::Ty) -> (Ty, Vec ty_from_ast(names, inner), TyKind::Path(path) => { let ty = match names.get(path.id) { - Some(&resolve::Res::Item(item)) => Ty::Udt(hir::Res::Item(item)), + Some(&resolve::Res::Item(item, _)) => Ty::Udt(hir::Res::Item(item)), Some(&resolve::Res::PrimTy(prim)) => Ty::Prim(prim), Some(resolve::Res::UnitTy) => Ty::Tuple(Vec::new()), // a path should never resolve to a parameter, diff --git a/compiler/qsc_frontend/src/typeck/rules.rs b/compiler/qsc_frontend/src/typeck/rules.rs index b3b8750bab..983be17c21 100644 --- a/compiler/qsc_frontend/src/typeck/rules.rs +++ b/compiler/qsc_frontend/src/typeck/rules.rs @@ -102,7 +102,7 @@ impl<'a> Context<'a> { TyKind::Hole => self.inferrer.fresh_ty(TySource::not_divergent(ty.span)), TyKind::Paren(inner) => self.infer_ty(inner), TyKind::Path(path) => match self.names.get(path.id) { - Some(&Res::Item(item)) => Ty::Udt(hir::Res::Item(item)), + Some(&Res::Item(item, _)) => Ty::Udt(hir::Res::Item(item)), Some(&Res::PrimTy(prim)) => Ty::Prim(prim), Some(Res::UnitTy) => Ty::Tuple(Vec::new()), None => Ty::Err, @@ -364,7 +364,7 @@ impl<'a> Context<'a> { ExprKind::Paren(expr) => self.infer_expr(expr), ExprKind::Path(path) => match self.names.get(path.id) { None => converge(Ty::Err), - Some(Res::Item(item)) => { + Some(Res::Item(item, _)) => { let scheme = self.globals.get(item).expect("item should have scheme"); let (ty, args) = self.inferrer.instantiate(scheme, expr.span); self.table.generics.insert(expr.id, args); diff --git a/compiler/qsc_frontend/src/typeck/tests.rs b/compiler/qsc_frontend/src/typeck/tests.rs index e1fd057c43..c2f83c1893 100644 --- a/compiler/qsc_frontend/src/typeck/tests.rs +++ b/compiler/qsc_frontend/src/typeck/tests.rs @@ -1111,7 +1111,7 @@ fn ternop_update_udt_unknown_field_name() { } "}, "", - &expect![[r#" + &expect![[r##" #19 79-81 "()" : Unit #21 87-155 "{\n let p = Pair(1, 2);\n let q = p w/ Third <- 3;\n }" : Unit #23 101-102 "p" : UDT @@ -1125,8 +1125,8 @@ fn ternop_update_udt_unknown_field_name() { #36 133-134 "p" : UDT #39 138-143 "Third" : ? #42 147-148 "3" : Int - Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), "Third", Span { lo: 133, hi: 148 })))) - "#]], + Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1) })), "Third", Span { lo: 133, hi: 148 })))) + "##]], ); } @@ -1146,7 +1146,7 @@ fn ternop_update_udt_unknown_field_name_known_global() { } "}, "", - &expect![[r#" + &expect![[r##" #19 81-83 "()" : Unit #21 89-91 "{}" : Unit #25 109-111 "()" : Unit @@ -1162,8 +1162,8 @@ fn ternop_update_udt_unknown_field_name_known_global() { #42 163-164 "p" : UDT #45 168-173 "Third" : ? #48 177-178 "3" : Int - Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), "Third", Span { lo: 163, hi: 178 })))) - "#]], + Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1) })), "Third", Span { lo: 163, hi: 178 })))) + "##]], ); } @@ -2011,15 +2011,15 @@ fn newtype_does_not_match_base_ty() { } "}, "", - &expect![[r#" + &expect![[r##" #12 56-58 "()" : Unit #16 65-78 "{ NewInt(5) }" : UDT #18 67-76 "NewInt(5)" : UDT #19 67-73 "NewInt" : (Int -> UDT) #22 73-76 "(5)" : Int #23 74-75 "5" : Int - Error(Type(Error(TyMismatch(Prim(Int), Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), Span { lo: 67, hi: 76 })))) - "#]], + Error(Type(Error(TyMismatch(Prim(Int), Udt(Item(ItemId { package: None, item: LocalItemId(1) })), Span { lo: 67, hi: 76 })))) + "##]], ); } @@ -2034,15 +2034,15 @@ fn newtype_does_not_match_other_newtype() { } "}, "", - &expect![[r#" + &expect![[r##" #18 84-86 "()" : Unit #22 97-111 "{ NewInt1(5) }" : UDT #24 99-109 "NewInt1(5)" : UDT #25 99-106 "NewInt1" : (Int -> UDT) #28 106-109 "(5)" : Int #29 107-108 "5" : Int - Error(Type(Error(TyMismatch(Udt(Item(ItemId { package: None, item: LocalItemId(2), status: Normal })), Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), Span { lo: 99, hi: 109 })))) - "#]], + Error(Type(Error(TyMismatch(Udt(Item(ItemId { package: None, item: LocalItemId(2) })), Udt(Item(ItemId { package: None, item: LocalItemId(1) })), Span { lo: 99, hi: 109 })))) + "##]], ); } @@ -2104,16 +2104,16 @@ fn newtype_field_invalid() { } "}, "", - &expect![[r#" + &expect![[r##" #13 59-68 "(x : Foo)" : UDT #14 60-67 "x : Foo" : UDT #20 74-106 "{\n let y = x::Nope;\n }" : Unit #22 88-89 "y" : ?1 #24 92-99 "x::Nope" : ?1 #25 92-93 "x" : UDT - Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), "Nope", Span { lo: 92, hi: 99 })))) + Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1) })), "Nope", Span { lo: 92, hi: 99 })))) Error(Type(Error(AmbiguousTy(Span { lo: 92, hi: 99 })))) - "#]], + "##]], ); } diff --git a/compiler/qsc_hir/src/global.rs b/compiler/qsc_hir/src/global.rs index 67cbffa386..11126580b8 100644 --- a/compiler/qsc_hir/src/global.rs +++ b/compiler/qsc_hir/src/global.rs @@ -13,6 +13,7 @@ pub struct Global { pub namespace: Rc, pub name: Rc, pub visibility: Visibility, + pub status: ItemStatus, pub kind: Kind, } @@ -94,14 +95,15 @@ impl PackageIter<'_> { let id = ItemId { package: self.id, item: item.id, - status: ItemStatus::from_attrs(item.attrs.as_ref()), }; + let status = ItemStatus::from_attrs(item.attrs.as_ref()); match (&item.kind, &parent) { (ItemKind::Callable(decl), Some(ItemKind::Namespace(namespace, _))) => Some(Global { namespace: Rc::clone(&namespace.name), name: Rc::clone(&decl.name.name), visibility: item.visibility, + status, kind: Kind::Term(Term { id, scheme: decl.scheme(), @@ -112,6 +114,7 @@ impl PackageIter<'_> { namespace: Rc::clone(&namespace.name), name: Rc::clone(&name.name), visibility: item.visibility, + status, kind: Kind::Term(Term { id, scheme: def.cons_scheme(id), @@ -122,6 +125,7 @@ impl PackageIter<'_> { namespace: Rc::clone(&namespace.name), name: Rc::clone(&name.name), visibility: item.visibility, + status, kind: Kind::Ty(Ty { id }), }) } @@ -129,6 +133,7 @@ impl PackageIter<'_> { namespace: "".into(), name: Rc::clone(&ident.name), visibility: Visibility::Public, + status, kind: Kind::Namespace, }), _ => None, diff --git a/compiler/qsc_hir/src/hir.rs b/compiler/qsc_hir/src/hir.rs index 07c4e8e0ec..949dc6594c 100644 --- a/compiler/qsc_hir/src/hir.rs +++ b/compiler/qsc_hir/src/hir.rs @@ -174,8 +174,6 @@ pub struct ItemId { pub package: Option, /// The item ID. pub item: LocalItemId, - /// The item status. - pub status: ItemStatus, } impl Display for ItemId { @@ -229,7 +227,6 @@ impl Res { Res::Item(id) if id.package.is_none() => Res::Item(ItemId { package: Some(package), item: id.item, - status: id.status, }), _ => *self, } diff --git a/compiler/qsc_passes/src/entry_point.rs b/compiler/qsc_passes/src/entry_point.rs index c1d7234121..784964a52f 100644 --- a/compiler/qsc_passes/src/entry_point.rs +++ b/compiler/qsc_passes/src/entry_point.rs @@ -10,8 +10,8 @@ use qsc_data_structures::span::Span; use qsc_hir::{ assigner::Assigner, hir::{ - Attr, CallableDecl, Expr, ExprKind, Item, ItemId, ItemKind, ItemStatus, LocalItemId, - Package, PatKind, Res, + Attr, CallableDecl, Expr, ExprKind, Item, ItemId, ItemKind, LocalItemId, Package, PatKind, + Res, }, ty::Ty, visit::Visitor, @@ -89,7 +89,6 @@ fn create_entry_from_callables( let item_id = ItemId { package: None, item, - status: ItemStatus::Normal, }; let callee = Expr { id: assigner.next_node(), diff --git a/language_service/src/name_locator.rs b/language_service/src/name_locator.rs index f520e78297..5713332935 100644 --- a/language_service/src/name_locator.rs +++ b/language_service/src/name_locator.rs @@ -155,7 +155,7 @@ impl<'inner, 'package, T: Handler<'package>> Visitor<'package> for Locator<'inne // and we want to do nothing. } ast::ItemKind::Ty(ident, def) => { - if let Some(resolve::Res::Item(item_id)) = + if let Some(resolve::Res::Item(item_id, _)) = self.compilation.user_unit.ast.names.get(ident.id) { let context = self.context.current_udt_id; @@ -270,7 +270,7 @@ impl<'inner, 'package, T: Handler<'package>> Visitor<'package> for Locator<'inne let res = self.compilation.user_unit.ast.names.get(path.id); if let Some(res) = res { match &res { - resolve::Res::Item(item_id) => { + resolve::Res::Item(item_id, _) => { let (item, package, resolved_item_id) = resolve_item_relative_to_user_package(self.compilation, item_id); match &item.kind { diff --git a/language_service/src/qsc_utils.rs b/language_service/src/qsc_utils.rs index 9b41614d2b..0c531250f8 100644 --- a/language_service/src/qsc_utils.rs +++ b/language_service/src/qsc_utils.rs @@ -4,7 +4,7 @@ use qsc::{ ast, compile::{self, Error}, - hir::{self, Item, ItemId, ItemStatus, Package, PackageId}, + hir::{self, Item, ItemId, Package, PackageId}, CompileUnit, PackageStore, PackageType, SourceMap, Span, TargetProfile, }; @@ -180,7 +180,6 @@ fn resolve_item<'a>( ItemId { package: package_id, item: item_id.item, - status: ItemStatus::Normal, }, ) } diff --git a/language_service/src/rename.rs b/language_service/src/rename.rs index 6d4d169af8..d90cee9ee6 100644 --- a/language_service/src/rename.rs +++ b/language_service/src/rename.rs @@ -102,7 +102,8 @@ impl<'a> Handler<'a> for Rename<'a> { name: &'a ast::Ident, _: &'a ast::CallableDecl, ) { - if let Some(resolve::Res::Item(item_id)) = self.compilation.user_unit.ast.names.get(name.id) + if let Some(resolve::Res::Item(item_id, _)) = + self.compilation.user_unit.ast.names.get(name.id) { self.get_spans_for_item_rename(item_id, name); } @@ -120,7 +121,7 @@ impl<'a> Handler<'a> for Rename<'a> { } fn at_new_type_def(&mut self, type_name: &'a ast::Ident, _: &'a ast::TyDef) { - if let Some(resolve::Res::Item(item_id)) = + if let Some(resolve::Res::Item(item_id, _)) = self.compilation.user_unit.ast.names.get(type_name.id) { self.get_spans_for_item_rename(item_id, type_name); diff --git a/language_service/src/signature_help.rs b/language_service/src/signature_help.rs index 5faf29ddc6..77413b3de7 100644 --- a/language_service/src/signature_help.rs +++ b/language_service/src/signature_help.rs @@ -344,7 +344,7 @@ fn try_get_direct_callee<'a>( callee: &ast::Expr, ) -> Option<(Option, &'a hir::CallableDecl, &'a str)> { if let ast::ExprKind::Path(path) = &*callee.kind { - if let Some(resolve::Res::Item(item_id)) = compilation.user_unit.ast.names.get(path.id) { + if let Some(resolve::Res::Item(item_id, _)) = compilation.user_unit.ast.names.get(path.id) { let (item, _, resolved_item_id) = resolve_item_relative_to_user_package(compilation, item_id); if let hir::ItemKind::Callable(callee_decl) = &item.kind { From 8b189da7decd92198f88d97ad8941a3cb89fca86 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Tue, 31 Oct 2023 11:36:17 -0700 Subject: [PATCH 4/7] Fix clippy failure --- compiler/qsc_frontend/src/typeck/tests.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/qsc_frontend/src/typeck/tests.rs b/compiler/qsc_frontend/src/typeck/tests.rs index c2f83c1893..01f17134a1 100644 --- a/compiler/qsc_frontend/src/typeck/tests.rs +++ b/compiler/qsc_frontend/src/typeck/tests.rs @@ -1111,7 +1111,7 @@ fn ternop_update_udt_unknown_field_name() { } "}, "", - &expect![[r##" + &expect![[r#" #19 79-81 "()" : Unit #21 87-155 "{\n let p = Pair(1, 2);\n let q = p w/ Third <- 3;\n }" : Unit #23 101-102 "p" : UDT @@ -1126,7 +1126,7 @@ fn ternop_update_udt_unknown_field_name() { #39 138-143 "Third" : ? #42 147-148 "3" : Int Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1) })), "Third", Span { lo: 133, hi: 148 })))) - "##]], + "#]], ); } @@ -1146,7 +1146,7 @@ fn ternop_update_udt_unknown_field_name_known_global() { } "}, "", - &expect![[r##" + &expect![[r#" #19 81-83 "()" : Unit #21 89-91 "{}" : Unit #25 109-111 "()" : Unit @@ -1163,7 +1163,7 @@ fn ternop_update_udt_unknown_field_name_known_global() { #45 168-173 "Third" : ? #48 177-178 "3" : Int Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1) })), "Third", Span { lo: 163, hi: 178 })))) - "##]], + "#]], ); } @@ -2011,7 +2011,7 @@ fn newtype_does_not_match_base_ty() { } "}, "", - &expect![[r##" + &expect![[r#" #12 56-58 "()" : Unit #16 65-78 "{ NewInt(5) }" : UDT #18 67-76 "NewInt(5)" : UDT @@ -2019,7 +2019,7 @@ fn newtype_does_not_match_base_ty() { #22 73-76 "(5)" : Int #23 74-75 "5" : Int Error(Type(Error(TyMismatch(Prim(Int), Udt(Item(ItemId { package: None, item: LocalItemId(1) })), Span { lo: 67, hi: 76 })))) - "##]], + "#]], ); } @@ -2034,7 +2034,7 @@ fn newtype_does_not_match_other_newtype() { } "}, "", - &expect![[r##" + &expect![[r#" #18 84-86 "()" : Unit #22 97-111 "{ NewInt1(5) }" : UDT #24 99-109 "NewInt1(5)" : UDT @@ -2042,7 +2042,7 @@ fn newtype_does_not_match_other_newtype() { #28 106-109 "(5)" : Int #29 107-108 "5" : Int Error(Type(Error(TyMismatch(Udt(Item(ItemId { package: None, item: LocalItemId(2) })), Udt(Item(ItemId { package: None, item: LocalItemId(1) })), Span { lo: 99, hi: 109 })))) - "##]], + "#]], ); } @@ -2104,7 +2104,7 @@ fn newtype_field_invalid() { } "}, "", - &expect![[r##" + &expect![[r#" #13 59-68 "(x : Foo)" : UDT #14 60-67 "x : Foo" : UDT #20 74-106 "{\n let y = x::Nope;\n }" : Unit @@ -2113,7 +2113,7 @@ fn newtype_field_invalid() { #25 92-93 "x" : UDT Error(Type(Error(MissingClassHasField(Udt(Item(ItemId { package: None, item: LocalItemId(1) })), "Nope", Span { lo: 92, hi: 99 })))) Error(Type(Error(AmbiguousTy(Span { lo: 92, hi: 99 })))) - "##]], + "#]], ); } From d2fa4156bf085afbad09b7804a2cdfccfa7a517d Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Fri, 3 Nov 2023 15:37:09 -0700 Subject: [PATCH 5/7] Update compiler/qsc_frontend/src/resolve.rs Co-authored-by: Scott Carda <55811729+ScottCarda-MS@users.noreply.github.com> --- compiler/qsc_frontend/src/resolve.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index 0bd8fb72fb..b1789436d0 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -420,11 +420,10 @@ impl AstVisitor<'_> for With<'_> { } fn visit_attr(&mut self, attr: &ast::Attr) { - if hir::Attr::from_str(attr.name.name.as_ref()) == Ok(hir::Attr::Config) { - // The Config attribute arguments do not go through name resolution. - return; + // The Config attribute arguments do not go through name resolution. + if hir::Attr::from_str(attr.name.name.as_ref()) != Ok(hir::Attr::Config) { + walk_attr(self, attr); } - walk_attr(self, attr); } fn visit_callable_decl(&mut self, decl: &ast::CallableDecl) { From 1b0b992bd3e758cf34ce2a9cc49f86f25023517c Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Thu, 9 Nov 2023 10:43:01 -0800 Subject: [PATCH 6/7] Fix changes lost in merge/rebase --- language_service/src/references.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/language_service/src/references.rs b/language_service/src/references.rs index 1085a0e6f1..fe1518aee6 100644 --- a/language_service/src/references.rs +++ b/language_service/src/references.rs @@ -50,7 +50,8 @@ impl<'a> Handler<'a> for ReferencesFinder<'a> { name: &'a ast::Ident, _: &'a ast::CallableDecl, ) { - if let Some(resolve::Res::Item(item_id)) = self.compilation.user_unit.ast.names.get(name.id) + if let Some(resolve::Res::Item(item_id, _)) = + self.compilation.user_unit.ast.names.get(name.id) { self.references = find_item_locations(item_id, self.compilation, self.include_declaration); @@ -69,7 +70,7 @@ impl<'a> Handler<'a> for ReferencesFinder<'a> { } fn at_new_type_def(&mut self, type_name: &'a ast::Ident, _: &'a ast::TyDef) { - if let Some(resolve::Res::Item(item_id)) = + if let Some(resolve::Res::Item(item_id, _)) = self.compilation.user_unit.ast.names.get(type_name.id) { self.references = @@ -258,7 +259,7 @@ struct FindItemRefs<'a> { impl<'a> Visitor<'_> for FindItemRefs<'a> { fn visit_path(&mut self, path: &'_ ast::Path) { let res = self.compilation.user_unit.ast.names.get(path.id); - if let Some(resolve::Res::Item(item_id)) = res { + if let Some(resolve::Res::Item(item_id, _)) = res { if *item_id == *self.item_id { self.locations.push(path.name.span); } @@ -268,7 +269,7 @@ impl<'a> Visitor<'_> for FindItemRefs<'a> { fn visit_ty(&mut self, ty: &'_ ast::Ty) { if let ast::TyKind::Path(ty_path) = &*ty.kind { let res = self.compilation.user_unit.ast.names.get(ty_path.id); - if let Some(resolve::Res::Item(item_id)) = res { + if let Some(resolve::Res::Item(item_id, _)) = res { if *item_id == *self.item_id { self.locations.push(ty_path.name.span); } From c94b34e649aae68dfb2228df7d2b830da82b3d4e Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Thu, 9 Nov 2023 12:36:56 -0800 Subject: [PATCH 7/7] Rename `Normal` to `Available` --- compiler/qsc_frontend/src/resolve.rs | 6 +++--- compiler/qsc_hir/src/hir.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index b1789436d0..d103ced165 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -629,7 +629,7 @@ fn bind_global_items( ) { names.insert( namespace.name.id, - Res::Item(intrapackage(assigner.next_item()), ItemStatus::Normal), + Res::Item(intrapackage(assigner.next_item()), ItemStatus::Available), ); scope.namespaces.insert(Rc::clone(&namespace.name.name)); @@ -817,7 +817,7 @@ fn resolve( if candidates.len() > 1 { // If there are multiple candidates, remove unimplemented items. This allows resolution to - // succeed in cases where both an older, unimplimented API and newer, implemented API with the + // succeed in cases where both an older, unimplemented API and newer, implemented API with the // same name are both in scope without forcing the user to fully qualify the name. let mut removals = Vec::new(); for res in candidates.keys() { @@ -878,7 +878,7 @@ fn resolve_scope_locals( } if let Some(&id) = scope.item(kind, name) { - return Some(Res::Item(id, ItemStatus::Normal)); + return Some(Res::Item(id, ItemStatus::Available)); } if let ScopeKind::Namespace(namespace) = &scope.kind { diff --git a/compiler/qsc_hir/src/hir.rs b/compiler/qsc_hir/src/hir.rs index 949dc6594c..3390d8786b 100644 --- a/compiler/qsc_hir/src/hir.rs +++ b/compiler/qsc_hir/src/hir.rs @@ -189,7 +189,7 @@ impl Display for ItemId { #[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] pub enum ItemStatus { /// The item is defined normally. - Normal, + Available, /// The item is marked as unimplemented and uses are disallowed. Unimplemented, } @@ -203,7 +203,7 @@ impl ItemStatus { return Self::Unimplemented; } } - Self::Normal + Self::Available } }