Skip to content

internal: Shrink mbe::ExpandError and mbe::ParseError #11403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions crates/hir_expand/src/builtin_fn_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ fn compile_error_expand(
let text = it.text.as_str();
if text.starts_with('"') && text.ends_with('"') {
// FIXME: does not handle raw strings
mbe::ExpandError::Other(text[1..text.len() - 1].to_string())
mbe::ExpandError::Other(text[1..text.len() - 1].into())
} else {
mbe::ExpandError::BindingError("`compile_error!` argument must be a string".into())
}
Expand Down Expand Up @@ -451,12 +451,12 @@ fn relative_file(
) -> Result<FileId, mbe::ExpandError> {
let call_site = call_id.as_file().original_file(db);
let path = AnchoredPath { anchor: call_site, path: path_str };
let res = db
.resolve_path(path)
.ok_or_else(|| mbe::ExpandError::Other(format!("failed to load file `{}`", path_str)))?;
let res = db.resolve_path(path).ok_or_else(|| {
mbe::ExpandError::Other(format!("failed to load file `{path_str}`").into())
})?;
// Prevent include itself
if res == call_site && !allow_recursion {
Err(mbe::ExpandError::Other(format!("recursive inclusion of `{}`", path_str)))
Err(mbe::ExpandError::Other(format!("recursive inclusion of `{path_str}`").into()))
} else {
Ok(res)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/hir_expand/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ fn macro_def(db: &dyn AstDatabase, id: MacroDefId) -> Result<Arc<TokenExpander>,
MacroDefKind::BuiltInEager(..) => {
// FIXME: Return a random error here just to make the types align.
// This obviously should do something real instead.
Err(mbe::ParseError::UnexpectedToken("unexpected eager macro".to_string()))
Err(mbe::ParseError::UnexpectedToken("unexpected eager macro".into()))
}
MacroDefKind::ProcMacro(expander, ..) => Ok(Arc::new(TokenExpander::ProcMacro(expander))),
}
Expand Down
4 changes: 2 additions & 2 deletions crates/hir_expand/src/proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ impl ProcMacroExpander {
{
ExpandResult {
value: tt.clone(),
err: Some(mbe::ExpandError::Other(text)),
err: Some(mbe::ExpandError::Other(text.into())),
}
}
ProcMacroExpansionError::System(text)
| ProcMacroExpansionError::Panic(text) => {
ExpandResult::only_err(mbe::ExpandError::Other(text))
ExpandResult::only_err(mbe::ExpandError::Other(text.into()))
}
},
}
Expand Down
30 changes: 12 additions & 18 deletions crates/mbe/src/expander/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,6 @@ impl Bindings {
}
}

macro_rules! err {
() => {
ExpandError::BindingError(format!(""))
};
($($tt:tt)*) => {
ExpandError::BindingError(format!($($tt)*))
};
}

#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub(super) struct Match {
pub(super) bindings: Bindings,
Expand Down Expand Up @@ -607,7 +598,7 @@ fn match_loop(pattern: &MetaTemplate, src: &tt::Subtree) -> Match {
src = it;
res.unmatched_tts += src.len();
}
res.add_err(err!("leftover tokens"));
res.add_err(ExpandError::binding_error("leftover tokens"));

if let Some(error_reover_item) = error_recover_item {
res.bindings = bindings_builder.build(&error_reover_item);
Expand Down Expand Up @@ -664,10 +655,9 @@ fn match_loop(pattern: &MetaTemplate, src: &tt::Subtree) -> Match {
}

fn match_leaf(lhs: &tt::Leaf, src: &mut TtIter) -> Result<(), ExpandError> {
let rhs = match src.expect_leaf() {
Ok(l) => l,
Err(()) => return Err(err!("expected leaf: `{}`", lhs)),
};
let rhs = src
.expect_leaf()
.map_err(|()| ExpandError::BindingError(format!("expected leaf: `{lhs}`").into()))?;
match (lhs, rhs) {
(
tt::Leaf::Punct(tt::Punct { char: lhs, .. }),
Expand Down Expand Up @@ -708,9 +698,13 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult<Option<Fragmen
"ident" => input
.expect_ident()
.map(|ident| tt::Leaf::from(ident.clone()).into())
.map_err(|()| err!("expected ident")),
"tt" => input.expect_tt().map_err(|()| err!()),
"lifetime" => input.expect_lifetime().map_err(|()| err!("expected lifetime")),
.map_err(|()| ExpandError::binding_error("expected ident")),
"tt" => input
.expect_tt()
.map_err(|()| ExpandError::binding_error("expected token tree")),
"lifetime" => input
.expect_lifetime()
.map_err(|()| ExpandError::binding_error("expected lifetime")),
"literal" => {
let neg = input.eat_char('-');
input
Expand All @@ -725,7 +719,7 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult<Option<Fragmen
}),
}
})
.map_err(|()| err!())
.map_err(|()| ExpandError::binding_error("expected literal"))
}
_ => Err(ExpandError::UnexpectedToken),
};
Expand Down
18 changes: 8 additions & 10 deletions crates/mbe/src/expander/transcriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,32 @@ impl Bindings {

fn get(&self, name: &str, nesting: &mut [NestingState]) -> Result<&Fragment, ExpandError> {
macro_rules! binding_err {
($($arg:tt)*) => { ExpandError::BindingError(format!($($arg)*)) };
($($arg:tt)*) => { ExpandError::BindingError(format!($($arg)*).into()) };
}

let mut b: &Binding = self
.inner
.get(name)
.ok_or_else(|| binding_err!("could not find binding `{}`", name))?;
let mut b: &Binding =
self.inner.get(name).ok_or_else(|| binding_err!("could not find binding `{name}`"))?;
for nesting_state in nesting.iter_mut() {
nesting_state.hit = true;
b = match b {
Binding::Fragment(_) => break,
Binding::Nested(bs) => bs.get(nesting_state.idx).ok_or_else(|| {
nesting_state.at_end = true;
binding_err!("could not find nested binding `{}`", name)
binding_err!("could not find nested binding `{name}`")
})?,
Binding::Empty => {
nesting_state.at_end = true;
return Err(binding_err!("could not find empty binding `{}`", name));
return Err(binding_err!("could not find empty binding `{name}`"));
}
};
}
match b {
Binding::Fragment(it) => Ok(it),
Binding::Nested(_) => {
Err(binding_err!("expected simple binding, found nested binding `{}`", name))
Err(binding_err!("expected simple binding, found nested binding `{name}`"))
}
Binding::Empty => {
Err(binding_err!("expected simple binding, found empty binding `{}`", name))
Err(binding_err!("expected simple binding, found empty binding `{name}`"))
}
}
}
Expand Down Expand Up @@ -180,7 +178,7 @@ fn expand_repeat(
);
return ExpandResult {
value: Fragment::Tokens(Subtree::default().into()),
err: Some(ExpandError::Other("Expand exceed limit".to_string())),
err: Some(ExpandError::Other("Expand exceed limit".into())),
};
}

Expand Down
81 changes: 44 additions & 37 deletions crates/mbe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,22 @@ pub use crate::{

#[derive(Debug, PartialEq, Eq, Clone)]
pub enum ParseError {
UnexpectedToken(String),
Expected(String),
UnexpectedToken(Box<str>),
Expected(Box<str>),
InvalidRepeat,
RepetitionEmptyTokenTree,
}

impl ParseError {
fn expected(e: &str) -> ParseError {
ParseError::Expected(e.into())
}

fn unexpected(e: &str) -> ParseError {
ParseError::UnexpectedToken(e.into())
}
}

impl fmt::Display for ParseError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Expand All @@ -58,11 +68,17 @@ impl fmt::Display for ParseError {
pub enum ExpandError {
NoMatchingRule,
UnexpectedToken,
BindingError(String),
BindingError(Box<str>),
ConversionError,
// FIXME: no way mbe should know about proc macros.
UnresolvedProcMacro,
Other(String),
Other(Box<str>),
}

impl ExpandError {
fn binding_error(e: &str) -> ExpandError {
ExpandError::BindingError(e.into())
}
}

impl fmt::Display for ExpandError {
Expand Down Expand Up @@ -107,28 +123,25 @@ impl Shift {

// Find the max token id inside a subtree
fn max_id(subtree: &tt::Subtree) -> Option<u32> {
subtree
.token_trees
.iter()
.filter_map(|tt| match tt {
tt::TokenTree::Subtree(subtree) => {
let tree_id = max_id(subtree);
match subtree.delimiter {
Some(it) if it.id != tt::TokenId::unspecified() => {
Some(tree_id.map_or(it.id.0, |t| t.max(it.id.0)))
}
_ => tree_id,
let filter = |tt: &_| match tt {
tt::TokenTree::Subtree(subtree) => {
let tree_id = max_id(subtree);
match subtree.delimiter {
Some(it) if it.id != tt::TokenId::unspecified() => {
Some(tree_id.map_or(it.id.0, |t| t.max(it.id.0)))
}
_ => tree_id,
}
tt::TokenTree::Leaf(leaf) => {
let &(tt::Leaf::Ident(tt::Ident { id, .. })
| tt::Leaf::Punct(tt::Punct { id, .. })
| tt::Leaf::Literal(tt::Literal { id, .. })) = leaf;
}
tt::TokenTree::Leaf(leaf) => {
let &(tt::Leaf::Ident(tt::Ident { id, .. })
| tt::Leaf::Punct(tt::Punct { id, .. })
| tt::Leaf::Literal(tt::Literal { id, .. })) = leaf;

(id != tt::TokenId::unspecified()).then(|| id.0)
}
})
.max()
(id != tt::TokenId::unspecified()).then(|| id.0)
}
};
subtree.token_trees.iter().filter_map(filter).max()
}
}

Expand Down Expand Up @@ -183,7 +196,7 @@ impl DeclarativeMacro {
rules.push(rule);
if let Err(()) = src.expect_char(';') {
if src.len() > 0 {
return Err(ParseError::Expected("expected `;`".to_string()));
return Err(ParseError::expected("expected `;`"));
}
break;
}
Expand All @@ -208,9 +221,7 @@ impl DeclarativeMacro {
rules.push(rule);
if let Err(()) = src.expect_any_char(&[';', ',']) {
if src.len() > 0 {
return Err(ParseError::Expected(
"expected `;` or `,` to delimit rules".to_string(),
));
return Err(ParseError::expected("expected `;` or `,` to delimit rules"));
}
break;
}
Expand All @@ -219,7 +230,7 @@ impl DeclarativeMacro {
cov_mark::hit!(parse_macro_def_simple);
let rule = Rule::parse(&mut src, false)?;
if src.len() != 0 {
return Err(ParseError::Expected("remaining tokens in macro def".to_string()));
return Err(ParseError::expected("remaining tokens in macro def"));
}
rules.push(rule);
}
Expand Down Expand Up @@ -256,16 +267,12 @@ impl DeclarativeMacro {

impl Rule {
fn parse(src: &mut TtIter, expect_arrow: bool) -> Result<Self, ParseError> {
let lhs = src
.expect_subtree()
.map_err(|()| ParseError::Expected("expected subtree".to_string()))?;
let lhs = src.expect_subtree().map_err(|()| ParseError::expected("expected subtree"))?;
if expect_arrow {
src.expect_char('=').map_err(|()| ParseError::Expected("expected `=`".to_string()))?;
src.expect_char('>').map_err(|()| ParseError::Expected("expected `>`".to_string()))?;
src.expect_char('=').map_err(|()| ParseError::expected("expected `=`"))?;
src.expect_char('>').map_err(|()| ParseError::expected("expected `>`"))?;
}
let rhs = src
.expect_subtree()
.map_err(|()| ParseError::Expected("expected subtree".to_string()))?;
let rhs = src.expect_subtree().map_err(|()| ParseError::expected("expected subtree"))?;

let lhs = MetaTemplate::parse_pattern(lhs)?;
let rhs = MetaTemplate::parse_template(rhs)?;
Expand Down Expand Up @@ -325,7 +332,7 @@ impl<T> ExpandResult<T> {
where
T: Default,
{
Self::only_err(ExpandError::Other(err))
Self::only_err(ExpandError::Other(err.into()))
}

pub fn map<U>(self, f: impl FnOnce(T) -> U) -> ExpandResult<U> {
Expand Down
14 changes: 5 additions & 9 deletions crates/mbe/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,6 @@ enum Mode {
Template,
}

macro_rules! err {
($($tt:tt)*) => {
ParseError::UnexpectedToken(($($tt)*).to_string())
};
}

fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Result<Op, ParseError> {
let res = match first {
tt::TokenTree::Leaf(leaf @ tt::Leaf::Punct(tt::Punct { char: '$', .. })) => {
Expand Down Expand Up @@ -142,7 +136,7 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul
Op::Var { name, kind, id }
}
tt::Leaf::Punct(_) | tt::Leaf::Literal(_) => {
return Err(ParseError::Expected("ident".to_string()))
return Err(ParseError::expected("expected ident"))
}
},
}
Expand All @@ -158,8 +152,10 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul

fn eat_fragment_kind(src: &mut TtIter<'_>, mode: Mode) -> Result<Option<SmolStr>, ParseError> {
if let Mode::Pattern = mode {
src.expect_char(':').map_err(|()| err!("missing fragment specifier"))?;
let ident = src.expect_ident().map_err(|()| err!("missing fragment specifier"))?;
src.expect_char(':').map_err(|()| ParseError::unexpected("missing fragment specifier"))?;
let ident = src
.expect_ident()
.map_err(|()| ParseError::unexpected("missing fragment specifier"))?;
return Ok(Some(ident.text.clone()));
};
Ok(None)
Expand Down
11 changes: 1 addition & 10 deletions crates/mbe/src/tt_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,6 @@ use tt::buffer::TokenBuffer;

use crate::{to_parser_input::to_parser_input, ExpandError, ExpandResult};

macro_rules! err {
() => {
ExpandError::BindingError(format!(""))
};
($($tt:tt)*) => {
ExpandError::BindingError(format!($($tt)*))
};
}

#[derive(Debug, Clone)]
pub(crate) struct TtIter<'a> {
pub(crate) inner: std::slice::Iter<'a, tt::TokenTree>,
Expand Down Expand Up @@ -115,7 +106,7 @@ impl<'a> TtIter<'a> {
}

let err = if error || !cursor.is_root() {
Some(err!("expected {:?}", entry_point))
Some(ExpandError::BindingError(format!("expected {entry_point:?}").into()))
} else {
None
};
Expand Down