From d13455c0c11d914405bb1c4ea9824d6bc7fb244c Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Tue, 10 Oct 2023 07:01:59 +0000 Subject: [PATCH 1/8] WIP: add Deprecated, Unimplemented attrs --- .../qsc_frontend/src/compile/preprocess.rs | 3 +- compiler/qsc_frontend/src/lower.rs | 62 ++++++---- compiler/qsc_frontend/src/resolve.rs | 114 ++++++++++++++---- compiler/qsc_frontend/src/resolve/tests.rs | 2 +- compiler/qsc_frontend/src/typeck/check.rs | 36 ++++-- compiler/qsc_frontend/src/typeck/convert.rs | 2 +- compiler/qsc_frontend/src/typeck/rules.rs | 4 +- compiler/qsc_hir/src/global.rs | 7 +- compiler/qsc_hir/src/hir.rs | 20 +++ 9 files changed, 194 insertions(+), 56 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/lower.rs b/compiler/qsc_frontend/src/lower.rs index bdb312d2fe..93e04d19b0 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)] @@ -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(hir::ItemId { item, .. })) => item, + Some(&resolve::Res::Item(hir::ItemId { item, .. }, _)) => item, _ => panic!("item should have item ID"), }; @@ -210,8 +210,8 @@ impl With<'_> { } 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 +219,43 @@ 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)); + Ok(hir::Attr::Deprecated(..)) => { + let arg = self.lower_expr(&attr.arg); + if !matches!(arg.ty, Ty::Arrow(..)) { + self.lowerer + .errors + .push(Error::InvalidAttrArgs("callable expression", attr.arg.span)); + } + Some(hir::Attr::Deprecated(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 } } @@ -704,7 +724,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 4695d87460..5ffade966c 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -7,18 +7,19 @@ 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, Attr, ItemId, LocalItemId, PackageId}, ty::{ParamId, Prim}, }; use std::{ collections::{hash_map::Entry, HashMap, HashSet}, rc::Rc, + str::FromStr, vec, }; use thiserror::Error; @@ -39,7 +40,7 @@ pub(super) type Names = IndexMap; #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub enum Res { /// A global item. - Item(ItemId), + Item(ItemId, ItemInfo), /// A local variable. Local(NodeId), /// A type/functor parameter in the generics section of the parent callable decl. @@ -50,6 +51,30 @@ pub enum Res { UnitTy, } +#[derive(Clone, Copy, Debug, Default, Eq, Hash, PartialEq)] +pub struct ItemInfo { + deprecated: Option, + unimplemented: bool, +} + +impl ItemInfo { + #[must_use] + pub fn new(attrs: &[Attr]) -> Self { + let mut info = Self { + deprecated: None, + unimplemented: false, + }; + for attr in attrs { + match attr { + Attr::Deprecated(span) => info.deprecated = Some(*span), + Attr::Unimplemented => info.unimplemented = true, + _ => {} + } + } + info + } +} + #[derive(Clone, Debug, Diagnostic, Error)] pub(super) enum Error { #[error("`{name}` could refer to the item in `{first_open}` or `{second_open}`")] @@ -77,6 +102,14 @@ pub(super) enum Error { span: Span, }, + #[error("use of deprecated item `{0}`")] + #[diagnostic(help( + "check the `Deprecated` attribute on the item definition for recommended alternatives" + ))] + #[diagnostic(severity(Warning))] + #[diagnostic(code("Qsc.Resolve.Deprecated"))] + Deprecated(String, #[label] Span), + #[error("duplicate declaration of `{0}` in namespace `{1}`")] #[diagnostic(code("Qsc.Resolve.Duplicate"))] Duplicate(String, String, #[label] Span), @@ -96,6 +129,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 { @@ -244,7 +282,12 @@ impl Resolver { 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((id, err)) => { + if let Some(err) = err { + self.errors.push(err); + } + self.names.insert(name.id, id); + } Err(err) => self.errors.push(err), } } @@ -253,7 +296,12 @@ 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((id, err)) => { + if let Some(err) = err { + self.errors.push(err); + } + self.names.insert(path.id, id); + } Err(err) => { if let Error::NotFound(name, span) = err { if let Some(dropped_name) = @@ -317,13 +365,15 @@ 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, ItemInfo::default())); 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, ItemInfo::default())); 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); @@ -393,8 +443,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 HashSet>) { @@ -570,14 +625,17 @@ impl GlobalTable { .tys .entry(global.namespace) .or_default() - .insert(global.name, Res::Item(ty.id)); + .insert(global.name, Res::Item(ty.id, ItemInfo::new(&global.attrs))); } 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, ItemInfo::new(&global.attrs)), + ); } global::Kind::Namespace => { self.scope.namespaces.insert(global.name); @@ -596,7 +654,7 @@ fn bind_global_items( ) { names.insert( namespace.name.id, - Res::Item(intrapackage(assigner.next_item())), + Res::Item(intrapackage(assigner.next_item()), ItemInfo::default()), ); scope.namespaces.insert(Rc::clone(&namespace.name.name)); @@ -639,7 +697,7 @@ fn is_field_update(globals: &GlobalScope, scopes: &[Scope], index: &ast::Expr) - let namespace = &path.namespace; resolve(NameKind::Term, globals, scopes, name, namespace) }, - Ok(Res::Local(_)) + Ok((Res::Local(_), _)) ), _ => false, } @@ -654,7 +712,7 @@ fn bind_global_item( ) -> Result<(), Error> { match &*item.kind { ast::ItemKind::Callable(decl) => { - let res = Res::Item(next_id()); + let res = Res::Item(next_id(), ItemInfo::default()); names.insert(decl.name.id, res); match scope .terms @@ -674,7 +732,7 @@ fn bind_global_item( } } ast::ItemKind::Ty(name, _) => { - let res = Res::Item(next_id()); + let res = Res::Item(next_id(), ItemInfo::default()); names.insert(name.id, res); match ( scope @@ -710,7 +768,7 @@ fn resolve( locals: &[Scope], name: &Ident, namespace: &Option>, -) -> Result { +) -> Result<(Res, Option), Error> { let mut candidates = HashMap::new(); let mut vars = true; let name_str = &(*name.name); @@ -719,7 +777,7 @@ fn resolve( if namespace.is_empty() { if let Some(res) = resolve_scope_locals(kind, globals, scope, vars, name_str) { // Local declarations shadow everything. - return Ok(res); + return Ok((res, None)); } } @@ -760,14 +818,14 @@ fn resolve( }); } if let Some((res, _)) = single(candidates) { - return Ok(res); + return Ok((res, None)); } } if candidates.is_empty() { if let Some(&res) = globals.get(kind, namespace, name_str) { // An unopened global is the last resort. - return Ok(res); + return Ok((res, None)); } } @@ -783,8 +841,20 @@ fn resolve( second_open_span: opens[1].span, }) } else { - single(candidates.into_keys()) - .ok_or_else(|| Error::NotFound(name_str.to_string(), name.span)) + let Some(res) = single(candidates.into_keys()) else { + return Err(Error::NotFound(name_str.to_string(), name.span)); + }; + match res { + Res::Item(_, info) if info.deprecated.is_some() => Ok(( + res, + Some(Error::Deprecated(name_str.to_string(), name.span)), + )), + Res::Item(_, info) if info.unimplemented => Ok(( + res, + Some(Error::Unimplemented(name_str.to_string(), name.span)), + )), + _ => Ok((res, None)), + } } } @@ -819,7 +889,7 @@ fn resolve_scope_locals( } if let Some(&id) = scope.item(kind, name) { - return Some(Res::Item(id)); + return Some(Res::Item(id, ItemInfo::default())); } if let ScopeKind::Namespace(namespace) = &scope.kind { 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 bb08b979f2..df36f853fa 100644 --- a/compiler/qsc_frontend/src/typeck/check.rs +++ b/compiler/qsc_frontend/src/typeck/check.rs @@ -11,15 +11,15 @@ use crate::{ typeck::convert::{self, MissingTyError}, }; use qsc_ast::{ - ast::{self, NodeId, TopLevelNode}, - visit::{self, Visitor}, + ast::{self, Expr, NodeId, TopLevelNode}, + visit::{self, walk_attr, Visitor}, }; use qsc_data_structures::index_map::IndexMap; use qsc_hir::{ hir::{self, ItemId, PackageId}, ty::{FunctorSetValue, Scheme, Ty, Udt}, }; -use std::{collections::HashMap, vec}; +use std::{collections::HashMap, str::FromStr, vec}; pub(crate) struct GlobalTable { udts: HashMap, @@ -175,6 +175,14 @@ impl Checker { )); } + fn check_attr_expr(&mut self, names: &Names, expr: &Expr) { + self.errors.extend( + &mut rules::expr(names, &self.globals, &mut self.table, expr) + .into_iter() + .filter(|e| !matches!(&e.0, ErrorKind::AmbiguousTy(_))), + ); + } + pub(crate) fn solve(&mut self, names: &Names) { self.errors.append(&mut rules::solve( names, @@ -201,7 +209,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"); }; @@ -216,7 +224,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"); }; @@ -245,8 +253,13 @@ impl Visitor<'_> for ItemCollector<'_> { visit::walk_item(self, item); } - // We do not typecheck attributes, as they are verified 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); + } } struct ItemChecker<'a> { @@ -265,4 +278,13 @@ impl Visitor<'_> for ItemChecker<'_> { self.checker.check_callable_decl(self.names, decl); visit::walk_callable_decl(self, decl); } + + fn visit_attr(&mut self, attr: &ast::Attr) { + if matches!( + hir::Attr::from_str(attr.name.name.as_ref()), + Ok(hir::Attr::Deprecated(_)) + ) { + self.checker.check_attr_expr(self.names, &attr.arg); + } + } } diff --git a/compiler/qsc_frontend/src/typeck/convert.rs b/compiler/qsc_frontend/src/typeck/convert.rs index 172beb6bb7..0a58c5a987 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 82bf34aa54..592683661e 100644 --- a/compiler/qsc_frontend/src/typeck/rules.rs +++ b/compiler/qsc_frontend/src/typeck/rules.rs @@ -101,7 +101,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, @@ -362,7 +362,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_hir/src/global.rs b/compiler/qsc_hir/src/global.rs index a085fc822d..dad9135818 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::{Attr, Item, ItemId, ItemKind, Package, PackageId, Visibility}, ty::Scheme, }; use qsc_data_structures::index_map; @@ -13,6 +13,7 @@ pub struct Global { pub name: Rc, pub visibility: Visibility, pub kind: Kind, + pub attrs: Vec, } pub enum Kind { @@ -104,6 +105,7 @@ impl PackageIter<'_> { id, scheme: decl.scheme(), }), + attrs: item.attrs.clone(), }), (ItemKind::Ty(name, def), Some(ItemKind::Namespace(namespace, _))) => { self.next = Some(Global { @@ -114,6 +116,7 @@ impl PackageIter<'_> { id, scheme: def.cons_scheme(id), }), + attrs: item.attrs.clone(), }); Some(Global { @@ -121,6 +124,7 @@ impl PackageIter<'_> { name: Rc::clone(&name.name), visibility: item.visibility, kind: Kind::Ty(Ty { id }), + attrs: item.attrs.clone(), }) } (ItemKind::Namespace(ident, _), None) => Some(Global { @@ -128,6 +132,7 @@ impl PackageIter<'_> { name: Rc::clone(&ident.name), visibility: Visibility::Public, kind: Kind::Namespace, + attrs: item.attrs.clone(), }), _ => None, } diff --git a/compiler/qsc_hir/src/hir.rs b/compiler/qsc_hir/src/hir.rs index 108f53c114..b65fae7c44 100644 --- a/compiler/qsc_hir/src/hir.rs +++ b/compiler/qsc_hir/src/hir.rs @@ -1126,8 +1126,28 @@ 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 has been deprecated and may eventually be removed. + Deprecated(Span), + /// 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), + "Deprecated" => Ok(Self::Deprecated(Span::default())), + "Unimplemented" => Ok(Self::Unimplemented), + _ => Err(()), + } + } } /// A field. From b184e32eb2f437828e4a2b554ce43908050cd305 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Wed, 11 Oct 2023 17:02:19 -0700 Subject: [PATCH 2/8] Allow empty deprecation attr --- compiler/qsc_frontend/src/lower.rs | 18 ++++++++++++------ compiler/qsc_frontend/src/resolve.rs | 15 ++++++++++----- compiler/qsc_hir/src/hir.rs | 4 ++-- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/compiler/qsc_frontend/src/lower.rs b/compiler/qsc_frontend/src/lower.rs index 93e04d19b0..95fac83863 100644 --- a/compiler/qsc_frontend/src/lower.rs +++ b/compiler/qsc_frontend/src/lower.rs @@ -242,14 +242,20 @@ impl With<'_> { } Ok(hir::Attr::Deprecated(..)) => { let arg = self.lower_expr(&attr.arg); - if !matches!(arg.ty, Ty::Arrow(..)) { - self.lowerer - .errors - .push(Error::InvalidAttrArgs("callable expression", attr.arg.span)); + + if matches!(arg.ty, Ty::Arrow(..)) { + Some(hir::Attr::Deprecated(Some(attr.arg.span))) + } else if matches!(arg.ty, Ty::Tuple(tup) if tup.is_empty()) { + Some(hir::Attr::Deprecated(None)) + } else { + self.lowerer.errors.push(Error::InvalidAttrArgs( + "callable expression or ()", + attr.arg.span, + )); + None } - Some(hir::Attr::Deprecated(attr.arg.span)) } - Err(_) => { + Err(()) => { self.lowerer.errors.push(Error::UnknownAttr( attr.name.name.to_string(), attr.name.span, diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index 5ffade966c..97b2ae488d 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -53,7 +53,8 @@ pub enum Res { #[derive(Clone, Copy, Debug, Default, Eq, Hash, PartialEq)] pub struct ItemInfo { - deprecated: Option, + deprecated: bool, + alternative: Option, unimplemented: bool, } @@ -61,12 +62,16 @@ impl ItemInfo { #[must_use] pub fn new(attrs: &[Attr]) -> Self { let mut info = Self { - deprecated: None, + deprecated: false, + alternative: None, unimplemented: false, }; for attr in attrs { match attr { - Attr::Deprecated(span) => info.deprecated = Some(*span), + Attr::Deprecated(span) => { + info.deprecated = true; + info.alternative = *span; + } Attr::Unimplemented => info.unimplemented = true, _ => {} } @@ -104,7 +109,7 @@ pub(super) enum Error { #[error("use of deprecated item `{0}`")] #[diagnostic(help( - "check the `Deprecated` attribute on the item definition for recommended alternatives" + "check the `Deprecated` attribute on the item definition for any recommended alternatives" ))] #[diagnostic(severity(Warning))] #[diagnostic(code("Qsc.Resolve.Deprecated"))] @@ -845,7 +850,7 @@ fn resolve( return Err(Error::NotFound(name_str.to_string(), name.span)); }; match res { - Res::Item(_, info) if info.deprecated.is_some() => Ok(( + Res::Item(_, info) if info.deprecated => Ok(( res, Some(Error::Deprecated(name_str.to_string(), name.span)), )), diff --git a/compiler/qsc_hir/src/hir.rs b/compiler/qsc_hir/src/hir.rs index b65fae7c44..487f2b10c8 100644 --- a/compiler/qsc_hir/src/hir.rs +++ b/compiler/qsc_hir/src/hir.rs @@ -1131,7 +1131,7 @@ pub enum Attr { /// Indicates that a callable is an entry point to a program. EntryPoint, /// Indicates that an item has been deprecated and may eventually be removed. - Deprecated(Span), + Deprecated(Option), /// Indicates that an item does not have an implementation available for use. Unimplemented, } @@ -1143,7 +1143,7 @@ impl FromStr for Attr { match s { "Config" => Ok(Self::Config), "EntryPoint" => Ok(Self::EntryPoint), - "Deprecated" => Ok(Self::Deprecated(Span::default())), + "Deprecated" => Ok(Self::Deprecated(None)), "Unimplemented" => Ok(Self::Unimplemented), _ => Err(()), } From 10076df915830aeffd1bf1a1abbb9e0068749462 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Wed, 11 Oct 2023 17:03:21 -0700 Subject: [PATCH 3/8] Update language service --- language_service/src/definition.rs | 2 +- language_service/src/hover.rs | 2 +- language_service/src/rename.rs | 10 +++++----- language_service/src/signature_help.rs | 3 ++- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/language_service/src/definition.rs b/language_service/src/definition.rs index 45195af04d..de67f24c13 100644 --- a/language_service/src/definition.rs +++ b/language_service/src/definition.rs @@ -185,7 +185,7 @@ impl<'a> Visitor<'a> for DefinitionFinder<'a> { let res = self.compilation.unit.ast.names.get(path.id); if let Some(res) = res { match &res { - resolve::Res::Item(item_id) => { + resolve::Res::Item(item_id, _) => { if let (Some(item), _) = find_item(self.compilation, item_id) { let lo = match &item.kind { hir::ItemKind::Callable(decl) => decl.name.span.lo, diff --git a/language_service/src/hover.rs b/language_service/src/hover.rs index 445d6db5f6..b83e7049f0 100644 --- a/language_service/src/hover.rs +++ b/language_service/src/hover.rs @@ -255,7 +255,7 @@ impl<'a> Visitor<'a> for HoverVisitor<'a> { let res = self.compilation.unit.ast.names.get(path.id); if let Some(res) = res { match &res { - resolve::Res::Item(item_id) => { + resolve::Res::Item(item_id, _) => { if let (Some(item), Some(package)) = find_item(self.compilation, item_id) { let ns = item .parent diff --git a/language_service/src/rename.rs b/language_service/src/rename.rs index 4dcde38932..61d3b9524e 100644 --- a/language_service/src/rename.rs +++ b/language_service/src/rename.rs @@ -153,7 +153,7 @@ impl<'a> Visitor<'a> for Rename<'a> { match &*item.kind { ast::ItemKind::Callable(decl) => { if span_touches(decl.name.span, self.offset) { - if let Some(resolve::Res::Item(item_id)) = + if let Some(resolve::Res::Item(item_id, _)) = self.compilation.unit.ast.names.get(decl.name.id) { self.get_spans_for_item_rename(item_id, &decl.name); @@ -173,7 +173,7 @@ impl<'a> Visitor<'a> for Rename<'a> { // 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.unit.ast.names.get(ident.id) { if span_touches(ident.span, self.offset) { @@ -253,7 +253,7 @@ impl<'a> Visitor<'a> for Rename<'a> { let res = self.compilation.unit.ast.names.get(path.id); if let Some(res) = res { match &res { - resolve::Res::Item(item_id) => { + resolve::Res::Item(item_id, _) => { self.get_spans_for_item_rename(item_id, &path.name); } resolve::Res::Local(node_id) => { @@ -275,7 +275,7 @@ struct ItemRename<'a> { impl<'a> Visitor<'_> for ItemRename<'a> { fn visit_path(&mut self, path: &'_ ast::Path) { let res = self.compilation.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); } @@ -285,7 +285,7 @@ impl<'a> Visitor<'_> for ItemRename<'a> { fn visit_ty(&mut self, ty: &'_ ast::Ty) { if let ast::TyKind::Path(ty_path) = &*ty.kind { let res = self.compilation.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); } diff --git a/language_service/src/signature_help.rs b/language_service/src/signature_help.rs index 45c5919376..77ea3a26ae 100644 --- a/language_service/src/signature_help.rs +++ b/language_service/src/signature_help.rs @@ -75,7 +75,8 @@ impl<'a> Visitor<'a> for SignatureHelpFinder<'a> { impl SignatureHelpFinder<'_> { fn process_direct_callee(&mut self, callee: &ast::Path, args: &ast::Expr) { - if let Some(resolve::Res::Item(item_id)) = self.compilation.unit.ast.names.get(callee.id) { + if let Some(resolve::Res::Item(item_id, _)) = self.compilation.unit.ast.names.get(callee.id) + { if let (Some(item), _) = find_item(self.compilation, item_id) { if let hir::ItemKind::Callable(callee) = &item.kind { // Check that the callee has parameters to give help for From 6b177a38076b6a6642a1c82689b8bece52d9fd04 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Mon, 16 Oct 2023 13:21:59 -0700 Subject: [PATCH 4/8] Avoid ambiguous error on deprecated/unimplemented --- compiler/qsc_frontend/src/resolve.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index 97b2ae488d..6e2b25aee8 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -834,6 +834,21 @@ fn resolve( } } + if candidates.len() > 1 { + // If there are multiple candidates, remove deprecated and unimplemented items. + let mut removals = Vec::new(); + for res in candidates.keys() { + if let Res::Item(_, info) = res { + if info.deprecated || info.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); From 51c4514e3818e8412bcd759c8e9b82d31574fc5b Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Tue, 17 Oct 2023 15:45:05 -0700 Subject: [PATCH 5/8] Add tests --- compiler/qsc_frontend/src/compile/tests.rs | 358 +++++++++++++++++++++ 1 file changed, 358 insertions(+) diff --git a/compiler/qsc_frontend/src/compile/tests.rs b/compiler/qsc_frontend/src/compile/tests.rs index d57f7c19aa..badd4d663d 100644 --- a/compiler/qsc_frontend/src/compile/tests.rs +++ b/compiler/qsc_frontend/src/compile/tests.rs @@ -783,3 +783,361 @@ fn two_files_error_eof() { Namespace (Ident 1 [26-29] "Bar"): "#]] .assert_eq(&unit.package.to_string()); } + +#[test] +fn deprecated_call_from_dependency_produces_error() { + let lib_sources = SourceMap::new( + [( + "lib".into(), + indoc! {" + namespace Foo { + @Deprecated() + 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( + Deprecated( + "Bar", + Span { + lo: 69, + hi: 72, + }, + ), + ), + ), + ] + "#]] + .assert_debug_eq(&unit.errors); +} + +#[test] +fn deprecated_attribute_with_callable_name_allowed() { + let sources = SourceMap::new( + [( + "test".into(), + indoc! {" + namespace Foo { + @Deprecated(Foo.Baz) + operation Bar() : Unit {} + + operation Baz() : Unit {} + } + "} + .into(), + )], + None, + ); + let store = PackageStore::new(super::core()); + let unit = compile(&store, &[], sources, TargetProfile::Full); + assert!(unit.errors.is_empty(), "{:#?}", unit.errors); +} + +#[test] +fn deprecated_attribute_with_lambda_allowed() { + let sources = SourceMap::new( + [( + "test".into(), + indoc! {" + namespace Foo { + @Deprecated(() => {}) + operation Bar() : Unit {} + } + "} + .into(), + )], + None, + ); + let store = PackageStore::new(super::core()); + let unit = compile(&store, &[], sources, TargetProfile::Full); + assert!(unit.errors.is_empty(), "{:#?}", unit.errors); +} + +#[test] +fn deprecated_attribute_call_within_unit_allowed() { + let sources = SourceMap::new( + [( + "test".into(), + indoc! {" + namespace Foo { + @Deprecated(() => {}) + operation Bar() : Unit {} + operation Baz() : Unit { + Bar(); + } + } + "} + .into(), + )], + None, + ); + let store = PackageStore::new(super::core()); + let unit = compile(&store, &[], sources, TargetProfile::Full); + assert!(unit.errors.is_empty(), "{:#?}", unit.errors); +} + +#[test] +fn deprecated_attribute_with_non_callable_expr_error() { + let sources = SourceMap::new( + [( + "test".into(), + indoc! {" + namespace Foo { + @Deprecated(1) + operation Bar() : Unit {} + } + "} + .into(), + )], + None, + ); + let store = PackageStore::new(super::core()); + let unit = compile(&store, &[], sources, TargetProfile::Full); + expect![[r#" + [ + Error( + Lower( + InvalidAttrArgs( + "callable expression or ()", + Span { + lo: 31, + hi: 34, + }, + ), + ), + ), + ] + "#]] + .assert_debug_eq(&unit.errors); +} + +#[test] +fn deprecated_attribute_avoids_ambiguous_error_with_duplicate_names_in_scope() { + let lib_sources = SourceMap::new( + [( + "lib".into(), + indoc! {" + namespace Foo { + @Deprecated() + 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); +} + +#[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_allowed() { + 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); + assert!(unit.errors.is_empty(), "{:#?}", 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); +} From f2b36bafa87ce0567bc75b3684aecdaa25839ade Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Tue, 17 Oct 2023 20:50:18 -0700 Subject: [PATCH 6/8] Rework item info tracking as item status --- compiler/qsc_frontend/src/compile/tests.rs | 5 +- compiler/qsc_frontend/src/lower.rs | 17 +++--- compiler/qsc_frontend/src/resolve.rs | 66 ++++++--------------- compiler/qsc_frontend/src/resolve/tests.rs | 2 +- compiler/qsc_frontend/src/typeck/check.rs | 12 ++-- 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 | 3 +- compiler/qsc_hir/src/hir.rs | 29 +++++++++ compiler/qsc_passes/src/entry_point.rs | 5 +- language_service/src/definition.rs | 2 +- language_service/src/hover.rs | 2 +- language_service/src/rename.rs | 10 ++-- language_service/src/signature_help.rs | 3 +- 15 files changed, 97 insertions(+), 95 deletions(-) diff --git a/compiler/qsc_frontend/src/compile/tests.rs b/compiler/qsc_frontend/src/compile/tests.rs index badd4d663d..c1daa4be09 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 ); diff --git a/compiler/qsc_frontend/src/lower.rs b/compiler/qsc_frontend/src/lower.rs index 95fac83863..db8667433c 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(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,7 +203,7 @@ impl With<'_> { kind, }); - Some(id) + Some(id.item) } fn lower_attr(&mut self, attr: &ast::Attr) -> Option { @@ -730,7 +727,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 6e2b25aee8..ca0f5be87d 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -13,7 +13,7 @@ use qsc_data_structures::{index_map::IndexMap, span::Span}; use qsc_hir::{ assigner::Assigner, global, - hir::{self, Attr, ItemId, LocalItemId, PackageId}, + hir::{self, ItemId, ItemStatus, LocalItemId, PackageId}, ty::{ParamId, Prim}, }; use std::{ @@ -40,7 +40,7 @@ pub(super) type Names = IndexMap; #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub enum Res { /// A global item. - Item(ItemId, ItemInfo), + Item(ItemId), /// A local variable. Local(NodeId), /// A type/functor parameter in the generics section of the parent callable decl. @@ -51,35 +51,6 @@ pub enum Res { UnitTy, } -#[derive(Clone, Copy, Debug, Default, Eq, Hash, PartialEq)] -pub struct ItemInfo { - deprecated: bool, - alternative: Option, - unimplemented: bool, -} - -impl ItemInfo { - #[must_use] - pub fn new(attrs: &[Attr]) -> Self { - let mut info = Self { - deprecated: false, - alternative: None, - unimplemented: false, - }; - for attr in attrs { - match attr { - Attr::Deprecated(span) => { - info.deprecated = true; - info.alternative = *span; - } - Attr::Unimplemented => info.unimplemented = true, - _ => {} - } - } - info - } -} - #[derive(Clone, Debug, Diagnostic, Error)] pub(super) enum Error { #[error("`{name}` could refer to the item in `{first_open}` or `{second_open}`")] @@ -370,15 +341,13 @@ 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, ItemInfo::default())); + self.names.insert(decl.name.id, Res::Item(id)); 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, ItemInfo::default())); + self.names.insert(name.id, Res::Item(id)); 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); @@ -630,17 +599,14 @@ impl GlobalTable { .tys .entry(global.namespace) .or_default() - .insert(global.name, Res::Item(ty.id, ItemInfo::new(&global.attrs))); + .insert(global.name, Res::Item(ty.id)); } global::Kind::Term(term) => { self.scope .terms .entry(global.namespace) .or_default() - .insert( - global.name, - Res::Item(term.id, ItemInfo::new(&global.attrs)), - ); + .insert(global.name, Res::Item(term.id)); } global::Kind::Namespace => { self.scope.namespaces.insert(global.name); @@ -659,7 +625,7 @@ fn bind_global_items( ) { names.insert( namespace.name.id, - Res::Item(intrapackage(assigner.next_item()), ItemInfo::default()), + Res::Item(intrapackage(assigner.next_item())), ); scope.namespaces.insert(Rc::clone(&namespace.name.name)); @@ -717,7 +683,7 @@ fn bind_global_item( ) -> Result<(), Error> { match &*item.kind { ast::ItemKind::Callable(decl) => { - let res = Res::Item(next_id(), ItemInfo::default()); + let res = Res::Item(next_id()); names.insert(decl.name.id, res); match scope .terms @@ -737,7 +703,7 @@ fn bind_global_item( } } ast::ItemKind::Ty(name, _) => { - let res = Res::Item(next_id(), ItemInfo::default()); + let res = Res::Item(next_id()); names.insert(name.id, res); match ( scope @@ -838,8 +804,11 @@ fn resolve( // If there are multiple candidates, remove deprecated and unimplemented items. let mut removals = Vec::new(); for res in candidates.keys() { - if let Res::Item(_, info) = res { - if info.deprecated || info.unimplemented { + if let Res::Item(item) = res { + if matches!( + item.status, + ItemStatus::Deprecated | ItemStatus::Unimplemented + ) { removals.push(*res); } } @@ -865,11 +834,11 @@ fn resolve( return Err(Error::NotFound(name_str.to_string(), name.span)); }; match res { - Res::Item(_, info) if info.deprecated => Ok(( + Res::Item(item) if item.status == ItemStatus::Deprecated => Ok(( res, Some(Error::Deprecated(name_str.to_string(), name.span)), )), - Res::Item(_, info) if info.unimplemented => Ok(( + Res::Item(item) if item.status == ItemStatus::Unimplemented => Ok(( res, Some(Error::Unimplemented(name_str.to_string(), name.span)), )), @@ -909,7 +878,7 @@ fn resolve_scope_locals( } if let Some(&id) = scope.item(kind, name) { - return Some(Res::Item(id, ItemInfo::default())); + return Some(Res::Item(id)); } if let ScopeKind::Namespace(namespace) = &scope.kind { @@ -957,6 +926,7 @@ 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 b8deecc13e..08622744c6 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 df36f853fa..6bfa4900c3 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 std::{collections::HashMap, str::FromStr, vec}; @@ -41,6 +41,7 @@ impl GlobalTable { let item_id = ItemId { package: Some(id), item: item.id, + status: ItemStatus::from_attrs(&item.attrs), }; match &item.kind { @@ -176,6 +177,8 @@ impl Checker { } fn check_attr_expr(&mut self, names: &Names, expr: &Expr) { + // To allow for lambdas in attribute arguments, we need to be more permissive + // about ambiguous types. We filter those errors out here. self.errors.extend( &mut rules::expr(names, &self.globals, &mut self.table, expr) .into_iter() @@ -209,7 +212,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"); }; @@ -224,7 +227,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"); }; @@ -255,7 +258,6 @@ impl Visitor<'_> for ItemCollector<'_> { 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); @@ -284,6 +286,8 @@ impl Visitor<'_> for ItemChecker<'_> { hir::Attr::from_str(attr.name.name.as_ref()), Ok(hir::Attr::Deprecated(_)) ) { + // Since the Deprecated attribute arguments can have lambdas, we go through + // item checking to ensure those lambdas get type checked. self.checker.check_attr_expr(self.names, &attr.arg); } } diff --git a/compiler/qsc_frontend/src/typeck/convert.rs b/compiler/qsc_frontend/src/typeck/convert.rs index 0a58c5a987..172beb6bb7 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 592683661e..82bf34aa54 100644 --- a/compiler/qsc_frontend/src/typeck/rules.rs +++ b/compiler/qsc_frontend/src/typeck/rules.rs @@ -101,7 +101,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, @@ -362,7 +362,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 01f17134a1..e5a0450bb2 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) })), "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 })))) + "##]], ); } @@ -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) })), "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 })))) + "##]], ); } @@ -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) })), 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 })))) + "##]], ); } @@ -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) })), 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 })))) + "##]], ); } @@ -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) })), "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 dad9135818..8eb106e6ff 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::{Attr, Item, ItemId, ItemKind, Package, PackageId, Visibility}, + hir::{Attr, 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 487f2b10c8..5f7f884797 100644 --- a/compiler/qsc_hir/src/hir.rs +++ b/compiler/qsc_hir/src/hir.rs @@ -176,6 +176,8 @@ pub struct ItemId { pub package: Option, /// The item ID. pub item: LocalItemId, + /// The item status. + pub status: ItemStatus, } impl Display for ItemId { @@ -187,6 +189,32 @@ 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 deprecated and uses are discouraged. + Deprecated, + /// 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 { + match attr { + Attr::Deprecated(_) => return Self::Deprecated, + 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)] @@ -207,6 +235,7 @@ impl Res { Res::Item(id) if id.package.is_none() => Res::Item(ItemId { package: Some(package), item: id.item, + status: id.status, }), Res::Item(id) if id.package.expect("none case should be handled above") != package => { panic!("should not try to update Res with existing package id") 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/definition.rs b/language_service/src/definition.rs index de67f24c13..45195af04d 100644 --- a/language_service/src/definition.rs +++ b/language_service/src/definition.rs @@ -185,7 +185,7 @@ impl<'a> Visitor<'a> for DefinitionFinder<'a> { let res = self.compilation.unit.ast.names.get(path.id); if let Some(res) = res { match &res { - resolve::Res::Item(item_id, _) => { + resolve::Res::Item(item_id) => { if let (Some(item), _) = find_item(self.compilation, item_id) { let lo = match &item.kind { hir::ItemKind::Callable(decl) => decl.name.span.lo, diff --git a/language_service/src/hover.rs b/language_service/src/hover.rs index b83e7049f0..445d6db5f6 100644 --- a/language_service/src/hover.rs +++ b/language_service/src/hover.rs @@ -255,7 +255,7 @@ impl<'a> Visitor<'a> for HoverVisitor<'a> { let res = self.compilation.unit.ast.names.get(path.id); if let Some(res) = res { match &res { - resolve::Res::Item(item_id, _) => { + resolve::Res::Item(item_id) => { if let (Some(item), Some(package)) = find_item(self.compilation, item_id) { let ns = item .parent diff --git a/language_service/src/rename.rs b/language_service/src/rename.rs index 61d3b9524e..4dcde38932 100644 --- a/language_service/src/rename.rs +++ b/language_service/src/rename.rs @@ -153,7 +153,7 @@ impl<'a> Visitor<'a> for Rename<'a> { match &*item.kind { ast::ItemKind::Callable(decl) => { if span_touches(decl.name.span, self.offset) { - if let Some(resolve::Res::Item(item_id, _)) = + if let Some(resolve::Res::Item(item_id)) = self.compilation.unit.ast.names.get(decl.name.id) { self.get_spans_for_item_rename(item_id, &decl.name); @@ -173,7 +173,7 @@ impl<'a> Visitor<'a> for Rename<'a> { // 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.unit.ast.names.get(ident.id) { if span_touches(ident.span, self.offset) { @@ -253,7 +253,7 @@ impl<'a> Visitor<'a> for Rename<'a> { let res = self.compilation.unit.ast.names.get(path.id); if let Some(res) = res { match &res { - resolve::Res::Item(item_id, _) => { + resolve::Res::Item(item_id) => { self.get_spans_for_item_rename(item_id, &path.name); } resolve::Res::Local(node_id) => { @@ -275,7 +275,7 @@ struct ItemRename<'a> { impl<'a> Visitor<'_> for ItemRename<'a> { fn visit_path(&mut self, path: &'_ ast::Path) { let res = self.compilation.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); } @@ -285,7 +285,7 @@ impl<'a> Visitor<'_> for ItemRename<'a> { fn visit_ty(&mut self, ty: &'_ ast::Ty) { if let ast::TyKind::Path(ty_path) = &*ty.kind { let res = self.compilation.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); } diff --git a/language_service/src/signature_help.rs b/language_service/src/signature_help.rs index 77ea3a26ae..45c5919376 100644 --- a/language_service/src/signature_help.rs +++ b/language_service/src/signature_help.rs @@ -75,8 +75,7 @@ impl<'a> Visitor<'a> for SignatureHelpFinder<'a> { impl SignatureHelpFinder<'_> { fn process_direct_callee(&mut self, callee: &ast::Path, args: &ast::Expr) { - if let Some(resolve::Res::Item(item_id, _)) = self.compilation.unit.ast.names.get(callee.id) - { + if let Some(resolve::Res::Item(item_id)) = self.compilation.unit.ast.names.get(callee.id) { if let (Some(item), _) = find_item(self.compilation, item_id) { if let hir::ItemKind::Callable(callee) = &item.kind { // Check that the callee has parameters to give help for From fdf2599f90a99e9a47ab474f2545e5cff25941e9 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Tue, 17 Oct 2023 21:04:32 -0700 Subject: [PATCH 7/8] Move error reporting up in stack --- compiler/qsc_frontend/src/resolve.rs | 50 +++++++++++------------ compiler/qsc_frontend/src/typeck/tests.rs | 20 ++++----- compiler/qsc_hir/src/global.rs | 7 +--- 3 files changed, 36 insertions(+), 41 deletions(-) diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index ca0f5be87d..3e1ec339b1 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -258,9 +258,15 @@ impl Resolver { fn resolve_ident(&mut self, kind: NameKind, name: &Ident) { let namespace = None; match resolve(kind, &self.globals, &self.scopes, name, &namespace) { - Ok((id, err)) => { - if let Some(err) = err { - self.errors.push(err); + Ok(id) => { + match id { + Res::Item(item) if item.status == ItemStatus::Deprecated => self + .errors + .push(Error::Deprecated(name.name.to_string(), name.span)), + Res::Item(item) if item.status == ItemStatus::Unimplemented => self + .errors + .push(Error::Unimplemented(name.name.to_string(), name.span)), + _ => {} } self.names.insert(name.id, id); } @@ -272,9 +278,15 @@ impl Resolver { let name = &path.name; let namespace = &path.namespace; match resolve(kind, &self.globals, &self.scopes, name, namespace) { - Ok((id, err)) => { - if let Some(err) = err { - self.errors.push(err); + Ok(id) => { + match id { + Res::Item(item) if item.status == ItemStatus::Deprecated => self + .errors + .push(Error::Deprecated(path.name.name.to_string(), path.span)), + Res::Item(item) if item.status == ItemStatus::Unimplemented => self + .errors + .push(Error::Unimplemented(path.name.name.to_string(), path.span)), + _ => {} } self.names.insert(path.id, id); } @@ -668,7 +680,7 @@ fn is_field_update(globals: &GlobalScope, scopes: &[Scope], index: &ast::Expr) - let namespace = &path.namespace; resolve(NameKind::Term, globals, scopes, name, namespace) }, - Ok((Res::Local(_), _)) + Ok(Res::Local(_)) ), _ => false, } @@ -739,7 +751,7 @@ fn resolve( locals: &[Scope], name: &Ident, namespace: &Option>, -) -> Result<(Res, Option), Error> { +) -> Result { let mut candidates = HashMap::new(); let mut vars = true; let name_str = &(*name.name); @@ -748,7 +760,7 @@ fn resolve( if namespace.is_empty() { if let Some(res) = resolve_scope_locals(kind, globals, scope, vars, name_str) { // Local declarations shadow everything. - return Ok((res, None)); + return Ok(res); } } @@ -789,14 +801,14 @@ fn resolve( }); } if let Some((res, _)) = single(candidates) { - return Ok((res, None)); + return Ok(res); } } if candidates.is_empty() { if let Some(&res) = globals.get(kind, namespace, name_str) { // An unopened global is the last resort. - return Ok((res, None)); + return Ok(res); } } @@ -830,20 +842,8 @@ fn resolve( second_open_span: opens[1].span, }) } else { - let Some(res) = single(candidates.into_keys()) else { - return Err(Error::NotFound(name_str.to_string(), name.span)); - }; - match res { - Res::Item(item) if item.status == ItemStatus::Deprecated => Ok(( - res, - Some(Error::Deprecated(name_str.to_string(), name.span)), - )), - Res::Item(item) if item.status == ItemStatus::Unimplemented => Ok(( - res, - Some(Error::Unimplemented(name_str.to_string(), name.span)), - )), - _ => Ok((res, None)), - } + single(candidates.into_keys()) + .ok_or_else(|| Error::NotFound(name_str.to_string(), name.span)) } } diff --git a/compiler/qsc_frontend/src/typeck/tests.rs b/compiler/qsc_frontend/src/typeck/tests.rs index e5a0450bb2..e1fd057c43 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), status: Normal })), "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), status: Normal })), "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), status: Normal })), 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), status: Normal })), Udt(Item(ItemId { package: None, item: LocalItemId(1), status: Normal })), 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), 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 8eb106e6ff..ffb6c60f98 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::{Attr, Item, ItemId, ItemKind, ItemStatus, Package, PackageId, Visibility}, + hir::{Item, ItemId, ItemKind, ItemStatus, Package, PackageId, Visibility}, ty::Scheme, }; use qsc_data_structures::index_map; @@ -13,7 +13,6 @@ pub struct Global { pub name: Rc, pub visibility: Visibility, pub kind: Kind, - pub attrs: Vec, } pub enum Kind { @@ -106,7 +105,6 @@ impl PackageIter<'_> { id, scheme: decl.scheme(), }), - attrs: item.attrs.clone(), }), (ItemKind::Ty(name, def), Some(ItemKind::Namespace(namespace, _))) => { self.next = Some(Global { @@ -117,7 +115,6 @@ impl PackageIter<'_> { id, scheme: def.cons_scheme(id), }), - attrs: item.attrs.clone(), }); Some(Global { @@ -125,7 +122,6 @@ impl PackageIter<'_> { name: Rc::clone(&name.name), visibility: item.visibility, kind: Kind::Ty(Ty { id }), - attrs: item.attrs.clone(), }) } (ItemKind::Namespace(ident, _), None) => Some(Global { @@ -133,7 +129,6 @@ impl PackageIter<'_> { name: Rc::clone(&ident.name), visibility: Visibility::Public, kind: Kind::Namespace, - attrs: item.attrs.clone(), }), _ => None, } From 8fb97f7f41e55d7ea795cdf13b4f838961b01a97 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Tue, 17 Oct 2023 21:20:27 -0700 Subject: [PATCH 8/8] Disallow unimplemented within unit --- compiler/qsc_frontend/src/compile/tests.rs | 19 +++++++- compiler/qsc_frontend/src/resolve.rs | 55 ++++++++++++---------- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/compiler/qsc_frontend/src/compile/tests.rs b/compiler/qsc_frontend/src/compile/tests.rs index c1daa4be09..4452be79df 100644 --- a/compiler/qsc_frontend/src/compile/tests.rs +++ b/compiler/qsc_frontend/src/compile/tests.rs @@ -1040,7 +1040,7 @@ fn unimplemented_call_from_dependency_produces_error() { } #[test] -fn unimplemented_attribute_call_within_unit_allowed() { +fn unimplemented_attribute_call_within_unit_error() { let sources = SourceMap::new( [( "test".into(), @@ -1059,7 +1059,22 @@ fn unimplemented_attribute_call_within_unit_allowed() { ); let store = PackageStore::new(super::core()); let unit = compile(&store, &[], sources, TargetProfile::Full); - assert!(unit.errors.is_empty(), "{:#?}", unit.errors); + expect![[r#" + [ + Error( + Resolve( + Unimplemented( + "Bar", + Span { + lo: 104, + hi: 107, + }, + ), + ), + ), + ] + "#]] + .assert_debug_eq(&unit.errors); } #[test] diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index 3e1ec339b1..0f10ac2f04 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -255,20 +255,24 @@ impl Resolver { } } + fn check_item_status(&mut self, res: Res, name: String, span: Span) { + match res { + Res::Item(item) if item.status == ItemStatus::Deprecated && item.package.is_some() => { + self.errors.push(Error::Deprecated(name, span)); + } + 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) => { - match id { - Res::Item(item) if item.status == ItemStatus::Deprecated => self - .errors - .push(Error::Deprecated(name.name.to_string(), name.span)), - Res::Item(item) if item.status == ItemStatus::Unimplemented => self - .errors - .push(Error::Unimplemented(name.name.to_string(), name.span)), - _ => {} - } - 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), } @@ -278,17 +282,9 @@ impl Resolver { let name = &path.name; let namespace = &path.namespace; match resolve(kind, &self.globals, &self.scopes, name, namespace) { - Ok(id) => { - match id { - Res::Item(item) if item.status == ItemStatus::Deprecated => self - .errors - .push(Error::Deprecated(path.name.name.to_string(), path.span)), - Res::Item(item) if item.status == ItemStatus::Unimplemented => self - .errors - .push(Error::Unimplemented(path.name.name.to_string(), path.span)), - _ => {} - } - 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 { @@ -686,6 +682,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, @@ -695,7 +698,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 @@ -715,7 +720,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