From 18f1a3c3c6ffc6179119e2503854807abbf9cd32 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 3 Dec 2023 18:50:29 +0100 Subject: [PATCH] Some final touches --- crates/hir-expand/src/lib.rs | 2 +- crates/hir-expand/src/quote.rs | 25 +++++++++++++--------- crates/hir-expand/src/span.rs | 3 +-- crates/mbe/src/benchmark.rs | 29 +++++++------------------- crates/mbe/src/expander/transcriber.rs | 17 +++++++++++---- crates/mbe/src/lib.rs | 2 +- crates/mbe/src/syntax_bridge.rs | 29 ++++++++++++++++---------- crates/mbe/src/token_map.rs | 24 ++++++++++++--------- crates/tt/src/lib.rs | 20 +++++++++++++----- crates/vfs/src/lib.rs | 2 +- 10 files changed, 86 insertions(+), 67 deletions(-) diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index 1d5b7dfa59..bc7b5fb211 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -663,7 +663,7 @@ impl ExpansionInfo { range: TextRange, ) -> Option<(FileRange, SyntaxContextId)> { debug_assert!(self.expanded.value.text_range().contains_range(range)); - let mut spans = self.exp_map.spans_for_node_range(range); + let mut spans = self.exp_map.spans_for_range(range); let SpanData { range, anchor, ctx } = spans.next()?; let mut start = range.start(); let mut end = range.end(); diff --git a/crates/hir-expand/src/quote.rs b/crates/hir-expand/src/quote.rs index 069bcc3bd8..0950f5d287 100644 --- a/crates/hir-expand/src/quote.rs +++ b/crates/hir-expand/src/quote.rs @@ -215,10 +215,18 @@ impl_to_to_tokentrees! { #[cfg(test)] mod tests { use crate::tt; - use ::tt::Span; + use base_db::{ + span::{SpanAnchor, SyntaxContextId, ROOT_ERASED_FILE_AST_ID}, + FileId, + }; use expect_test::expect; + use syntax::{TextRange, TextSize}; - const DUMMY: tt::SpanData = tt::SpanData::DUMMY; + const DUMMY: tt::SpanData = tt::SpanData { + range: TextRange::empty(TextSize::new(0)), + anchor: SpanAnchor { file_id: FileId::BOGUS, ast_id: ROOT_ERASED_FILE_AST_ID }, + ctx: SyntaxContextId::ROOT, + }; #[test] fn test_quote_delimiters() { @@ -242,10 +250,7 @@ mod tests { } fn mk_ident(name: &str) -> crate::tt::Ident { - crate::tt::Ident { - text: name.into(), - span: ::DUMMY, - } + crate::tt::Ident { text: name.into(), span: DUMMY } } #[test] @@ -256,8 +261,8 @@ mod tests { assert_eq!(quoted.to_string(), "hello"); let t = format!("{quoted:?}"); expect![[r#" - SUBTREE $$ SpanData { range: 0..0, anchor: SpanAnchor(FileId(0), 0), ctx: SyntaxContextId(0) } SpanData { range: 0..0, anchor: SpanAnchor(FileId(0), 0), ctx: SyntaxContextId(0) } - IDENT hello SpanData { range: 0..0, anchor: SpanAnchor(FileId(0), 0), ctx: SyntaxContextId(0) }"#]].assert_eq(&t); + SUBTREE $$ SpanData { range: 0..0, anchor: SpanAnchor(FileId(4294967295), 0), ctx: SyntaxContextId(0) } SpanData { range: 0..0, anchor: SpanAnchor(FileId(4294967295), 0), ctx: SyntaxContextId(0) } + IDENT hello SpanData { range: 0..0, anchor: SpanAnchor(FileId(4294967295), 0), ctx: SyntaxContextId(0) }"#]].assert_eq(&t); } #[test] @@ -290,8 +295,8 @@ mod tests { let list = crate::tt::Subtree { delimiter: crate::tt::Delimiter { kind: crate::tt::DelimiterKind::Brace, - open: ::DUMMY, - close: ::DUMMY, + open: DUMMY, + close: DUMMY, }, token_trees: fields.collect(), }; diff --git a/crates/hir-expand/src/span.rs b/crates/hir-expand/src/span.rs index 1703923d11..0a6c22fe42 100644 --- a/crates/hir-expand/src/span.rs +++ b/crates/hir-expand/src/span.rs @@ -4,13 +4,12 @@ use base_db::{ span::{ErasedFileAstId, SpanAnchor, SpanData, SyntaxContextId, ROOT_ERASED_FILE_AST_ID}, FileId, }; -use mbe::TokenMap; use syntax::{ast::HasModuleItem, AstNode, TextRange, TextSize}; use triomphe::Arc; use crate::db::ExpandDatabase; -pub type ExpansionSpanMap = TokenMap; +pub type ExpansionSpanMap = mbe::SpanMap; /// Spanmap for a macro file or a real file #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/crates/mbe/src/benchmark.rs b/crates/mbe/src/benchmark.rs index 271efe1a92..f503aecce2 100644 --- a/crates/mbe/src/benchmark.rs +++ b/crates/mbe/src/benchmark.rs @@ -6,11 +6,10 @@ use syntax::{ AstNode, SmolStr, }; use test_utils::{bench, bench_fixture, skip_slow_tests}; -use tt::Span; use crate::{ parser::{MetaVarKind, Op, RepeatKind, Separator}, - syntax_node_to_token_tree, DeclarativeMacro, DummyTestSpanData, DummyTestSpanMap, + syntax_node_to_token_tree, DeclarativeMacro, DummyTestSpanData, DummyTestSpanMap, DUMMY, }; #[test] @@ -97,8 +96,8 @@ fn invocation_fixtures( loop { let mut subtree = tt::Subtree { delimiter: tt::Delimiter { - open: DummyTestSpanData::DUMMY, - close: DummyTestSpanData::DUMMY, + open: DUMMY, + close: DUMMY, kind: tt::DelimiterKind::Invisible, }, token_trees: vec![], @@ -211,34 +210,20 @@ fn invocation_fixtures( *seed } fn make_ident(ident: &str) -> tt::TokenTree { - tt::Leaf::Ident(tt::Ident { span: DummyTestSpanData::DUMMY, text: SmolStr::new(ident) }) - .into() + tt::Leaf::Ident(tt::Ident { span: DUMMY, text: SmolStr::new(ident) }).into() } fn make_punct(char: char) -> tt::TokenTree { - tt::Leaf::Punct(tt::Punct { - span: DummyTestSpanData::DUMMY, - char, - spacing: tt::Spacing::Alone, - }) - .into() + tt::Leaf::Punct(tt::Punct { span: DUMMY, char, spacing: tt::Spacing::Alone }).into() } fn make_literal(lit: &str) -> tt::TokenTree { - tt::Leaf::Literal(tt::Literal { - span: DummyTestSpanData::DUMMY, - text: SmolStr::new(lit), - }) - .into() + tt::Leaf::Literal(tt::Literal { span: DUMMY, text: SmolStr::new(lit) }).into() } fn make_subtree( kind: tt::DelimiterKind, token_trees: Option>>, ) -> tt::TokenTree { tt::Subtree { - delimiter: tt::Delimiter { - open: DummyTestSpanData::DUMMY, - close: DummyTestSpanData::DUMMY, - kind, - }, + delimiter: tt::Delimiter { open: DUMMY, close: DUMMY, kind }, token_trees: token_trees.unwrap_or_default(), } .into() diff --git a/crates/mbe/src/expander/transcriber.rs b/crates/mbe/src/expander/transcriber.rs index e4a46c1659..7a3e8653c2 100644 --- a/crates/mbe/src/expander/transcriber.rs +++ b/crates/mbe/src/expander/transcriber.rs @@ -79,8 +79,8 @@ impl Bindings { } MetaVarKind::Block => Fragment::Tokens(tt::TokenTree::Subtree(tt::Subtree { delimiter: tt::Delimiter { - open: S::DUMMY, - close: S::DUMMY, + open: span, + close: span, kind: tt::DelimiterKind::Brace, }, token_trees: vec![], @@ -225,6 +225,7 @@ fn expand_subtree( arena.push( tt::Leaf::Literal(tt::Literal { text: index.to_string().into(), + // FIXME span: S::DUMMY, }) .into(), @@ -282,8 +283,12 @@ fn expand_subtree( } }; arena.push( - tt::Leaf::Literal(tt::Literal { text: c.to_string().into(), span: S::DUMMY }) - .into(), + tt::Leaf::Literal(tt::Literal { + text: c.to_string().into(), + // FIXME + span: S::DUMMY, + }) + .into(), ); } } @@ -337,7 +342,9 @@ fn expand_var( } Err(e) => ExpandResult { value: Fragment::Tokens(tt::TokenTree::Subtree(tt::Subtree::empty(tt::DelimSpan { + // FIXME open: S::DUMMY, + // FIXME close: S::DUMMY, }))), err: Some(e), @@ -479,6 +486,7 @@ fn fix_up_and_push_path_tt(buf: &mut Vec>, subtree: tt tt::Leaf::Punct(tt::Punct { char: ':', spacing: tt::Spacing::Joint, + // FIXME span: S::DUMMY, }) .into(), @@ -487,6 +495,7 @@ fn fix_up_and_push_path_tt(buf: &mut Vec>, subtree: tt tt::Leaf::Punct(tt::Punct { char: ':', spacing: tt::Spacing::Alone, + // FIXME span: S::DUMMY, }) .into(), diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs index 10eb59bb83..2b52e04b25 100644 --- a/crates/mbe/src/lib.rs +++ b/crates/mbe/src/lib.rs @@ -38,7 +38,7 @@ pub use crate::{ syntax_node_to_token_tree, syntax_node_to_token_tree_modified, token_tree_to_syntax_node, SpanMapper, }, - token_map::TokenMap, + token_map::SpanMap, }; pub use crate::syntax_bridge::dummy_test_span_utils::*; diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs index 5722a5bd8e..1c46471a38 100644 --- a/crates/mbe/src/syntax_bridge.rs +++ b/crates/mbe/src/syntax_bridge.rs @@ -13,7 +13,7 @@ use tt::{ Span, SpanData, SyntaxContext, }; -use crate::{to_parser_input::to_parser_input, tt_iter::TtIter, TokenMap}; +use crate::{to_parser_input::to_parser_input, tt_iter::TtIter, SpanMap}; #[cfg(test)] mod tests; @@ -22,7 +22,7 @@ pub trait SpanMapper { fn span_for(&self, range: TextRange) -> S; } -impl SpanMapper for TokenMap { +impl SpanMapper for SpanMap { fn span_for(&self, range: TextRange) -> S { self.span_at(range.start()) } @@ -34,10 +34,12 @@ impl> SpanMapper for &SM { } } +/// Dummy things for testing where spans don't matter. pub(crate) mod dummy_test_span_utils { use super::*; pub type DummyTestSpanData = tt::SpanData; + pub const DUMMY: DummyTestSpanData = DummyTestSpanData::DUMMY; #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct DummyTestSpanAnchor; @@ -62,9 +64,8 @@ pub(crate) mod dummy_test_span_utils { } } -/// Convert the syntax node to a `TokenTree` (what macro -/// will consume). -/// FIXME: Flesh out the doc comment more thoroughly +/// Converts a syntax tree to a [`tt::Subtree`] using the provided span map to populate the +/// subtree's spans. pub fn syntax_node_to_token_tree( node: &SyntaxNode, map: SpanMap, @@ -79,6 +80,9 @@ where convert_tokens(&mut c) } +/// Converts a syntax tree to a [`tt::Subtree`] using the provided span map to populate the +/// subtree's spans. Additionally using the append and remove parameters, the additional tokens can +/// be injected or hidden from the output. pub fn syntax_node_to_token_tree_modified( node: &SyntaxNode, map: SpanMap, @@ -107,10 +111,12 @@ where // * AssocItems(SmallVec<[ast::AssocItem; 1]>) // * ForeignItems(SmallVec<[ast::ForeignItem; 1]> +/// Converts a [`tt::Subtree`] back to a [`SyntaxNode`]. +/// The produced `SpanMap` contains a mapping from the syntax nodes offsets to the subtree's spans. pub fn token_tree_to_syntax_node( tt: &tt::Subtree>, entry_point: parser::TopEntryPoint, -) -> (Parse, TokenMap>) +) -> (Parse, SpanMap>) where SpanData: Span, Anchor: Copy, @@ -142,7 +148,8 @@ where tree_sink.finish() } -/// Convert a string to a `TokenTree` +/// Convert a string to a `TokenTree`. The spans of the subtree will be anchored to the provided +/// anchor with the given context. pub fn parse_to_token_tree( anchor: Anchor, ctx: Ctx, @@ -161,7 +168,7 @@ where Some(convert_tokens(&mut conv)) } -/// Convert a string to a `TokenTree` +/// Convert a string to a `TokenTree`. The passed span will be used for all spans of the produced subtree. pub fn parse_to_token_tree_static_span(span: S, text: &str) -> Option> where S: Span, @@ -798,7 +805,7 @@ where cursor: Cursor<'a, SpanData>, text_pos: TextSize, inner: SyntaxTreeBuilder, - token_map: TokenMap>, + token_map: SpanMap>, } impl<'a, Anchor, Ctx> TtTreeSink<'a, Anchor, Ctx> @@ -811,11 +818,11 @@ where cursor, text_pos: 0.into(), inner: SyntaxTreeBuilder::default(), - token_map: TokenMap::empty(), + token_map: SpanMap::empty(), } } - fn finish(mut self) -> (Parse, TokenMap>) { + fn finish(mut self) -> (Parse, SpanMap>) { self.token_map.finish(); (self.inner.finish(), self.token_map) } diff --git a/crates/mbe/src/token_map.rs b/crates/mbe/src/token_map.rs index 5871c0f125..b51d7575a1 100644 --- a/crates/mbe/src/token_map.rs +++ b/crates/mbe/src/token_map.rs @@ -8,30 +8,33 @@ use tt::Span; /// Maps absolute text ranges for the corresponding file to the relevant span data. #[derive(Debug, PartialEq, Eq, Clone, Hash)] -// FIXME: Rename to SpanMap -pub struct TokenMap { - // FIXME: This needs to be sorted by (FileId, AstId) - // Then we can do a binary search on the file id, - // then a bin search on the ast id? +pub struct SpanMap { spans: Vec<(TextSize, S)>, } -impl TokenMap { +impl SpanMap { + /// Creates a new empty [`SpanMap`]. pub fn empty() -> Self { Self { spans: Vec::new() } } + /// Finalizes the [`SpanMap`], shrinking its backing storage and validating that the offsets are + /// in order. pub fn finish(&mut self) { assert!(self.spans.iter().tuple_windows().all(|(a, b)| a.0 < b.0)); self.spans.shrink_to_fit(); } + /// Pushes a new span onto the [`SpanMap`]. pub fn push(&mut self, offset: TextSize, span: S) { + debug_assert!(self.spans.last().map_or(true, |&(last_offset, _)| last_offset < offset)); self.spans.push((offset, span)); } + /// Returns all [`TextRange`]s that correspond to the given span. + /// + /// Note this does a linear search through the entire backing vector. pub fn ranges_with_span(&self, span: S) -> impl Iterator + '_ { - // FIXME: linear search self.spans.iter().enumerate().filter_map(move |(idx, &(end, s))| { if s != span { return None; @@ -41,14 +44,15 @@ impl TokenMap { }) } - // FIXME: We need APIs for fetching the span of a token as well as for a whole node. The node - // one *is* fallible though. + /// Returns the span at the given position. pub fn span_at(&self, offset: TextSize) -> S { let entry = self.spans.partition_point(|&(it, _)| it <= offset); self.spans[entry].1 } - pub fn spans_for_node_range(&self, range: TextRange) -> impl Iterator + '_ { + /// Returns the spans associated with the given range. + /// In other words, this will return all spans that correspond to all offsets within the given range. + pub fn spans_for_range(&self, range: TextRange) -> impl Iterator + '_ { let (start, end) = (range.start(), range.end()); let start_entry = self.spans.partition_point(|&(it, _)| it <= start); let end_entry = self.spans[start_entry..].partition_point(|&(it, _)| it <= end); // FIXME: this might be wrong? diff --git a/crates/tt/src/lib.rs b/crates/tt/src/lib.rs index b1f2185162..10e0586218 100644 --- a/crates/tt/src/lib.rs +++ b/crates/tt/src/lib.rs @@ -23,6 +23,7 @@ pub struct SpanData { } impl Span for SpanData { + #[allow(deprecated)] const DUMMY: Self = SpanData { range: TextRange::empty(TextSize::new(0)), anchor: Anchor::DUMMY, @@ -30,18 +31,24 @@ impl Span for SpanData { }; } +pub trait Span: std::fmt::Debug + Copy + Sized + Eq { + // FIXME: Should not exist. Dummy spans will always be wrong if they leak somewhere. Instead, + // the call site or def site spans should be used in relevant places, its just that we don't + // expose those everywhere in the yet. + const DUMMY: Self; +} + +// FIXME: Should not exist pub trait SpanAnchor: std::fmt::Debug + Copy + Sized + Eq + Copy + fmt::Debug + std::hash::Hash { + #[deprecated(note = "this should not exist")] const DUMMY: Self; } -// FIXME: Get rid of this trait? -pub trait Span: std::fmt::Debug + Copy + Sized + Eq { - const DUMMY: Self; -} - +// FIXME: Should not exist pub trait SyntaxContext: std::fmt::Debug + Copy + Sized + Eq { + #[deprecated(note = "this should not exist")] const DUMMY: Self; } @@ -128,6 +135,7 @@ pub struct DelimSpan { } impl DelimSpan { + // FIXME should not exist pub const DUMMY: Self = Self { open: S::DUMMY, close: S::DUMMY }; } @@ -139,9 +147,11 @@ pub struct Delimiter { } impl Delimiter { + // FIXME should not exist pub const DUMMY_INVISIBLE: Self = Self { open: S::DUMMY, close: S::DUMMY, kind: DelimiterKind::Invisible }; + // FIXME should not exist pub const fn dummy_invisible() -> Self { Self::DUMMY_INVISIBLE } diff --git a/crates/vfs/src/lib.rs b/crates/vfs/src/lib.rs index c6866bbfb9..128559727b 100644 --- a/crates/vfs/src/lib.rs +++ b/crates/vfs/src/lib.rs @@ -63,7 +63,7 @@ pub use paths::{AbsPath, AbsPathBuf}; pub struct FileId(pub u32); impl FileId { - /// Think twice about using this. If this ends up in a wrong place it will cause panics! + /// Think twice about using this outside of tests. If this ends up in a wrong place it will cause panics! pub const BOGUS: FileId = FileId(u32::MAX); }