Skip to content

Commit 6162d12

Browse files
bors[bot]Veykril
andauthored
Merge #11403
11403: internal: Shrink `mbe::ExpandError` and `mbe::ParseError` r=Veykril a=Veykril Also fixes #10051, as we no longer emit an empty diagnostic in some expansion cases which seems to trip up vscode for some reason. Using `compile_error!("")` will still trigger the vscode bug. bors r+ Co-authored-by: Lukas Wirth <[email protected]>
2 parents 6ef6fa0 + 2ad71f1 commit 6162d12

File tree

8 files changed

+78
-92
lines changed

8 files changed

+78
-92
lines changed

crates/hir_expand/src/builtin_fn_macro.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ fn compile_error_expand(
368368
let text = it.text.as_str();
369369
if text.starts_with('"') && text.ends_with('"') {
370370
// FIXME: does not handle raw strings
371-
mbe::ExpandError::Other(text[1..text.len() - 1].to_string())
371+
mbe::ExpandError::Other(text[1..text.len() - 1].into())
372372
} else {
373373
mbe::ExpandError::BindingError("`compile_error!` argument must be a string".into())
374374
}
@@ -451,12 +451,12 @@ fn relative_file(
451451
) -> Result<FileId, mbe::ExpandError> {
452452
let call_site = call_id.as_file().original_file(db);
453453
let path = AnchoredPath { anchor: call_site, path: path_str };
454-
let res = db
455-
.resolve_path(path)
456-
.ok_or_else(|| mbe::ExpandError::Other(format!("failed to load file `{}`", path_str)))?;
454+
let res = db.resolve_path(path).ok_or_else(|| {
455+
mbe::ExpandError::Other(format!("failed to load file `{path_str}`").into())
456+
})?;
457457
// Prevent include itself
458458
if res == call_site && !allow_recursion {
459-
Err(mbe::ExpandError::Other(format!("recursive inclusion of `{}`", path_str)))
459+
Err(mbe::ExpandError::Other(format!("recursive inclusion of `{path_str}`").into()))
460460
} else {
461461
Ok(res)
462462
}

crates/hir_expand/src/db.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ fn macro_def(db: &dyn AstDatabase, id: MacroDefId) -> Result<Arc<TokenExpander>,
390390
MacroDefKind::BuiltInEager(..) => {
391391
// FIXME: Return a random error here just to make the types align.
392392
// This obviously should do something real instead.
393-
Err(mbe::ParseError::UnexpectedToken("unexpected eager macro".to_string()))
393+
Err(mbe::ParseError::UnexpectedToken("unexpected eager macro".into()))
394394
}
395395
MacroDefKind::ProcMacro(expander, ..) => Ok(Arc::new(TokenExpander::ProcMacro(expander))),
396396
}

crates/hir_expand/src/proc_macro.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,12 @@ impl ProcMacroExpander {
5151
{
5252
ExpandResult {
5353
value: tt.clone(),
54-
err: Some(mbe::ExpandError::Other(text)),
54+
err: Some(mbe::ExpandError::Other(text.into())),
5555
}
5656
}
5757
ProcMacroExpansionError::System(text)
5858
| ProcMacroExpansionError::Panic(text) => {
59-
ExpandResult::only_err(mbe::ExpandError::Other(text))
59+
ExpandResult::only_err(mbe::ExpandError::Other(text.into()))
6060
}
6161
},
6262
}

crates/mbe/src/expander/matcher.rs

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,6 @@ impl Bindings {
8888
}
8989
}
9090

91-
macro_rules! err {
92-
() => {
93-
ExpandError::BindingError(format!(""))
94-
};
95-
($($tt:tt)*) => {
96-
ExpandError::BindingError(format!($($tt)*))
97-
};
98-
}
99-
10091
#[derive(Clone, Debug, Default, PartialEq, Eq)]
10192
pub(super) struct Match {
10293
pub(super) bindings: Bindings,
@@ -607,7 +598,7 @@ fn match_loop(pattern: &MetaTemplate, src: &tt::Subtree) -> Match {
607598
src = it;
608599
res.unmatched_tts += src.len();
609600
}
610-
res.add_err(err!("leftover tokens"));
601+
res.add_err(ExpandError::binding_error("leftover tokens"));
611602

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

666657
fn match_leaf(lhs: &tt::Leaf, src: &mut TtIter) -> Result<(), ExpandError> {
667-
let rhs = match src.expect_leaf() {
668-
Ok(l) => l,
669-
Err(()) => return Err(err!("expected leaf: `{}`", lhs)),
670-
};
658+
let rhs = src
659+
.expect_leaf()
660+
.map_err(|()| ExpandError::BindingError(format!("expected leaf: `{lhs}`").into()))?;
671661
match (lhs, rhs) {
672662
(
673663
tt::Leaf::Punct(tt::Punct { char: lhs, .. }),
@@ -708,9 +698,13 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult<Option<Fragmen
708698
"ident" => input
709699
.expect_ident()
710700
.map(|ident| tt::Leaf::from(ident.clone()).into())
711-
.map_err(|()| err!("expected ident")),
712-
"tt" => input.expect_tt().map_err(|()| err!()),
713-
"lifetime" => input.expect_lifetime().map_err(|()| err!("expected lifetime")),
701+
.map_err(|()| ExpandError::binding_error("expected ident")),
702+
"tt" => input
703+
.expect_tt()
704+
.map_err(|()| ExpandError::binding_error("expected token tree")),
705+
"lifetime" => input
706+
.expect_lifetime()
707+
.map_err(|()| ExpandError::binding_error("expected lifetime")),
714708
"literal" => {
715709
let neg = input.eat_char('-');
716710
input
@@ -725,7 +719,7 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult<Option<Fragmen
725719
}),
726720
}
727721
})
728-
.map_err(|()| err!())
722+
.map_err(|()| ExpandError::binding_error("expected literal"))
729723
}
730724
_ => Err(ExpandError::UnexpectedToken),
731725
};

crates/mbe/src/expander/transcriber.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,34 +17,32 @@ impl Bindings {
1717

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

23-
let mut b: &Binding = self
24-
.inner
25-
.get(name)
26-
.ok_or_else(|| binding_err!("could not find binding `{}`", name))?;
23+
let mut b: &Binding =
24+
self.inner.get(name).ok_or_else(|| binding_err!("could not find binding `{name}`"))?;
2725
for nesting_state in nesting.iter_mut() {
2826
nesting_state.hit = true;
2927
b = match b {
3028
Binding::Fragment(_) => break,
3129
Binding::Nested(bs) => bs.get(nesting_state.idx).ok_or_else(|| {
3230
nesting_state.at_end = true;
33-
binding_err!("could not find nested binding `{}`", name)
31+
binding_err!("could not find nested binding `{name}`")
3432
})?,
3533
Binding::Empty => {
3634
nesting_state.at_end = true;
37-
return Err(binding_err!("could not find empty binding `{}`", name));
35+
return Err(binding_err!("could not find empty binding `{name}`"));
3836
}
3937
};
4038
}
4139
match b {
4240
Binding::Fragment(it) => Ok(it),
4341
Binding::Nested(_) => {
44-
Err(binding_err!("expected simple binding, found nested binding `{}`", name))
42+
Err(binding_err!("expected simple binding, found nested binding `{name}`"))
4543
}
4644
Binding::Empty => {
47-
Err(binding_err!("expected simple binding, found empty binding `{}`", name))
45+
Err(binding_err!("expected simple binding, found empty binding `{name}`"))
4846
}
4947
}
5048
}
@@ -180,7 +178,7 @@ fn expand_repeat(
180178
);
181179
return ExpandResult {
182180
value: Fragment::Tokens(Subtree::default().into()),
183-
err: Some(ExpandError::Other("Expand exceed limit".to_string())),
181+
err: Some(ExpandError::Other("Expand exceed limit".into())),
184182
};
185183
}
186184

crates/mbe/src/lib.rs

Lines changed: 44 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,22 @@ pub use crate::{
3737

3838
#[derive(Debug, PartialEq, Eq, Clone)]
3939
pub enum ParseError {
40-
UnexpectedToken(String),
41-
Expected(String),
40+
UnexpectedToken(Box<str>),
41+
Expected(Box<str>),
4242
InvalidRepeat,
4343
RepetitionEmptyTokenTree,
4444
}
4545

46+
impl ParseError {
47+
fn expected(e: &str) -> ParseError {
48+
ParseError::Expected(e.into())
49+
}
50+
51+
fn unexpected(e: &str) -> ParseError {
52+
ParseError::UnexpectedToken(e.into())
53+
}
54+
}
55+
4656
impl fmt::Display for ParseError {
4757
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
4858
match self {
@@ -58,11 +68,17 @@ impl fmt::Display for ParseError {
5868
pub enum ExpandError {
5969
NoMatchingRule,
6070
UnexpectedToken,
61-
BindingError(String),
71+
BindingError(Box<str>),
6272
ConversionError,
6373
// FIXME: no way mbe should know about proc macros.
6474
UnresolvedProcMacro,
65-
Other(String),
75+
Other(Box<str>),
76+
}
77+
78+
impl ExpandError {
79+
fn binding_error(e: &str) -> ExpandError {
80+
ExpandError::BindingError(e.into())
81+
}
6682
}
6783

6884
impl fmt::Display for ExpandError {
@@ -107,28 +123,25 @@ impl Shift {
107123

108124
// Find the max token id inside a subtree
109125
fn max_id(subtree: &tt::Subtree) -> Option<u32> {
110-
subtree
111-
.token_trees
112-
.iter()
113-
.filter_map(|tt| match tt {
114-
tt::TokenTree::Subtree(subtree) => {
115-
let tree_id = max_id(subtree);
116-
match subtree.delimiter {
117-
Some(it) if it.id != tt::TokenId::unspecified() => {
118-
Some(tree_id.map_or(it.id.0, |t| t.max(it.id.0)))
119-
}
120-
_ => tree_id,
126+
let filter = |tt: &_| match tt {
127+
tt::TokenTree::Subtree(subtree) => {
128+
let tree_id = max_id(subtree);
129+
match subtree.delimiter {
130+
Some(it) if it.id != tt::TokenId::unspecified() => {
131+
Some(tree_id.map_or(it.id.0, |t| t.max(it.id.0)))
121132
}
133+
_ => tree_id,
122134
}
123-
tt::TokenTree::Leaf(leaf) => {
124-
let &(tt::Leaf::Ident(tt::Ident { id, .. })
125-
| tt::Leaf::Punct(tt::Punct { id, .. })
126-
| tt::Leaf::Literal(tt::Literal { id, .. })) = leaf;
135+
}
136+
tt::TokenTree::Leaf(leaf) => {
137+
let &(tt::Leaf::Ident(tt::Ident { id, .. })
138+
| tt::Leaf::Punct(tt::Punct { id, .. })
139+
| tt::Leaf::Literal(tt::Literal { id, .. })) = leaf;
127140

128-
(id != tt::TokenId::unspecified()).then(|| id.0)
129-
}
130-
})
131-
.max()
141+
(id != tt::TokenId::unspecified()).then(|| id.0)
142+
}
143+
};
144+
subtree.token_trees.iter().filter_map(filter).max()
132145
}
133146
}
134147

@@ -183,7 +196,7 @@ impl DeclarativeMacro {
183196
rules.push(rule);
184197
if let Err(()) = src.expect_char(';') {
185198
if src.len() > 0 {
186-
return Err(ParseError::Expected("expected `;`".to_string()));
199+
return Err(ParseError::expected("expected `;`"));
187200
}
188201
break;
189202
}
@@ -208,9 +221,7 @@ impl DeclarativeMacro {
208221
rules.push(rule);
209222
if let Err(()) = src.expect_any_char(&[';', ',']) {
210223
if src.len() > 0 {
211-
return Err(ParseError::Expected(
212-
"expected `;` or `,` to delimit rules".to_string(),
213-
));
224+
return Err(ParseError::expected("expected `;` or `,` to delimit rules"));
214225
}
215226
break;
216227
}
@@ -219,7 +230,7 @@ impl DeclarativeMacro {
219230
cov_mark::hit!(parse_macro_def_simple);
220231
let rule = Rule::parse(&mut src, false)?;
221232
if src.len() != 0 {
222-
return Err(ParseError::Expected("remaining tokens in macro def".to_string()));
233+
return Err(ParseError::expected("remaining tokens in macro def"));
223234
}
224235
rules.push(rule);
225236
}
@@ -256,16 +267,12 @@ impl DeclarativeMacro {
256267

257268
impl Rule {
258269
fn parse(src: &mut TtIter, expect_arrow: bool) -> Result<Self, ParseError> {
259-
let lhs = src
260-
.expect_subtree()
261-
.map_err(|()| ParseError::Expected("expected subtree".to_string()))?;
270+
let lhs = src.expect_subtree().map_err(|()| ParseError::expected("expected subtree"))?;
262271
if expect_arrow {
263-
src.expect_char('=').map_err(|()| ParseError::Expected("expected `=`".to_string()))?;
264-
src.expect_char('>').map_err(|()| ParseError::Expected("expected `>`".to_string()))?;
272+
src.expect_char('=').map_err(|()| ParseError::expected("expected `=`"))?;
273+
src.expect_char('>').map_err(|()| ParseError::expected("expected `>`"))?;
265274
}
266-
let rhs = src
267-
.expect_subtree()
268-
.map_err(|()| ParseError::Expected("expected subtree".to_string()))?;
275+
let rhs = src.expect_subtree().map_err(|()| ParseError::expected("expected subtree"))?;
269276

270277
let lhs = MetaTemplate::parse_pattern(lhs)?;
271278
let rhs = MetaTemplate::parse_template(rhs)?;
@@ -325,7 +332,7 @@ impl<T> ExpandResult<T> {
325332
where
326333
T: Default,
327334
{
328-
Self::only_err(ExpandError::Other(err))
335+
Self::only_err(ExpandError::Other(err.into()))
329336
}
330337

331338
pub fn map<U>(self, f: impl FnOnce(T) -> U) -> ExpandResult<U> {

crates/mbe/src/parser.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,6 @@ enum Mode {
104104
Template,
105105
}
106106

107-
macro_rules! err {
108-
($($tt:tt)*) => {
109-
ParseError::UnexpectedToken(($($tt)*).to_string())
110-
};
111-
}
112-
113107
fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Result<Op, ParseError> {
114108
let res = match first {
115109
tt::TokenTree::Leaf(leaf @ tt::Leaf::Punct(tt::Punct { char: '$', .. })) => {
@@ -142,7 +136,7 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul
142136
Op::Var { name, kind, id }
143137
}
144138
tt::Leaf::Punct(_) | tt::Leaf::Literal(_) => {
145-
return Err(ParseError::Expected("ident".to_string()))
139+
return Err(ParseError::expected("expected ident"))
146140
}
147141
},
148142
}
@@ -158,8 +152,10 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul
158152

159153
fn eat_fragment_kind(src: &mut TtIter<'_>, mode: Mode) -> Result<Option<SmolStr>, ParseError> {
160154
if let Mode::Pattern = mode {
161-
src.expect_char(':').map_err(|()| err!("missing fragment specifier"))?;
162-
let ident = src.expect_ident().map_err(|()| err!("missing fragment specifier"))?;
155+
src.expect_char(':').map_err(|()| ParseError::unexpected("missing fragment specifier"))?;
156+
let ident = src
157+
.expect_ident()
158+
.map_err(|()| ParseError::unexpected("missing fragment specifier"))?;
163159
return Ok(Some(ident.text.clone()));
164160
};
165161
Ok(None)

crates/mbe/src/tt_iter.rs

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,6 @@ use tt::buffer::TokenBuffer;
66

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

9-
macro_rules! err {
10-
() => {
11-
ExpandError::BindingError(format!(""))
12-
};
13-
($($tt:tt)*) => {
14-
ExpandError::BindingError(format!($($tt)*))
15-
};
16-
}
17-
189
#[derive(Debug, Clone)]
1910
pub(crate) struct TtIter<'a> {
2011
pub(crate) inner: std::slice::Iter<'a, tt::TokenTree>,
@@ -115,7 +106,7 @@ impl<'a> TtIter<'a> {
115106
}
116107

117108
let err = if error || !cursor.is_root() {
118-
Some(err!("expected {:?}", entry_point))
109+
Some(ExpandError::BindingError(format!("expected {entry_point:?}").into()))
119110
} else {
120111
None
121112
};

0 commit comments

Comments
 (0)