From d3d32feef37668d178710613bfcab7a2f6a47386 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Mon, 30 Oct 2023 15:52:09 -0700 Subject: [PATCH] Introduce Unimplimented 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 bdb312d2fe..7b9c52bfea 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 d138cce89c..69e88106d8 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 e51929234f..340281fff2 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, } @@ -1121,8 +1146,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 0cc9e437f3..772c9073b9 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, }; @@ -142,6 +142,7 @@ fn resolve_item<'a>( ItemId { package: package_id, item: item_id.item, + status: ItemStatus::Normal, }, ) }