-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Remove various has_errors
or err_count
uses
#120342
Changes from 1 commit
f68741b
f6f0e04
646c8fc
2b60e56
3042da0
054e1e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -485,7 +485,9 @@ pub fn compile_declarative_macro( | |
) | ||
.pop() | ||
.unwrap(); | ||
valid &= check_lhs_nt_follows(sess, def, &tt); | ||
// We don't handle errors here, the driver will abort | ||
// after parsing/expansion. we can report every error in every macro this way. | ||
valid &= check_lhs_nt_follows(sess, def, &tt).is_ok(); | ||
return tt; | ||
} | ||
sess.dcx().span_bug(def.span, "wrong-structured lhs") | ||
|
@@ -589,18 +591,19 @@ pub fn compile_declarative_macro( | |
(mk_syn_ext(expander), rule_spans) | ||
} | ||
|
||
fn check_lhs_nt_follows(sess: &Session, def: &ast::Item, lhs: &mbe::TokenTree) -> bool { | ||
fn check_lhs_nt_follows( | ||
sess: &Session, | ||
def: &ast::Item, | ||
lhs: &mbe::TokenTree, | ||
) -> Result<(), ErrorGuaranteed> { | ||
// lhs is going to be like TokenTree::Delimited(...), where the | ||
// entire lhs is those tts. Or, it can be a "bare sequence", not wrapped in parens. | ||
if let mbe::TokenTree::Delimited(.., delimited) = lhs { | ||
check_matcher(sess, def, &delimited.tts) | ||
} else { | ||
let msg = "invalid macro matcher; matchers must be contained in balanced delimiters"; | ||
sess.dcx().span_err(lhs.span(), msg); | ||
false | ||
Err(sess.dcx().span_err(lhs.span(), msg)) | ||
} | ||
// we don't abort on errors on rejection, the driver will do that for us | ||
// after parsing/expansion. we can report every error in every macro this way. | ||
} | ||
|
||
fn is_empty_token_tree(sess: &Session, seq: &mbe::SequenceRepetition) -> bool { | ||
|
@@ -675,12 +678,15 @@ fn check_rhs(sess: &Session, rhs: &mbe::TokenTree) -> bool { | |
false | ||
} | ||
|
||
fn check_matcher(sess: &Session, def: &ast::Item, matcher: &[mbe::TokenTree]) -> bool { | ||
fn check_matcher( | ||
sess: &Session, | ||
def: &ast::Item, | ||
matcher: &[mbe::TokenTree], | ||
) -> Result<(), ErrorGuaranteed> { | ||
let first_sets = FirstSets::new(matcher); | ||
let empty_suffix = TokenSet::empty(); | ||
let err = sess.dcx().err_count(); | ||
check_matcher_core(sess, def, &first_sets, matcher, &empty_suffix); | ||
err == sess.dcx().err_count() | ||
check_matcher_core(sess, def, &first_sets, matcher, &empty_suffix)?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can drop the trailing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function returns Result<()> while the _core function returns an ok value There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
Ok(()) | ||
} | ||
|
||
fn has_compile_error_macro(rhs: &mbe::TokenTree) -> bool { | ||
|
@@ -1020,11 +1026,13 @@ fn check_matcher_core<'tt>( | |
first_sets: &FirstSets<'tt>, | ||
matcher: &'tt [mbe::TokenTree], | ||
follow: &TokenSet<'tt>, | ||
) -> TokenSet<'tt> { | ||
) -> Result<TokenSet<'tt>, ErrorGuaranteed> { | ||
use mbe::TokenTree; | ||
|
||
let mut last = TokenSet::empty(); | ||
|
||
let mut errored = Ok(()); | ||
|
||
// 2. For each token and suffix [T, SUFFIX] in M: | ||
// ensure that T can be followed by SUFFIX, and if SUFFIX may be empty, | ||
// then ensure T can also be followed by any element of FOLLOW. | ||
|
@@ -1068,7 +1076,7 @@ fn check_matcher_core<'tt>( | |
token::CloseDelim(d.delim), | ||
span.close, | ||
)); | ||
check_matcher_core(sess, def, first_sets, &d.tts, &my_suffix); | ||
check_matcher_core(sess, def, first_sets, &d.tts, &my_suffix)?; | ||
// don't track non NT tokens | ||
last.replace_with_irrelevant(); | ||
|
||
|
@@ -1100,7 +1108,7 @@ fn check_matcher_core<'tt>( | |
// At this point, `suffix_first` is built, and | ||
// `my_suffix` is some TokenSet that we can use | ||
// for checking the interior of `seq_rep`. | ||
let next = check_matcher_core(sess, def, first_sets, &seq_rep.tts, my_suffix); | ||
let next = check_matcher_core(sess, def, first_sets, &seq_rep.tts, my_suffix)?; | ||
if next.maybe_empty { | ||
last.add_all(&next); | ||
} else { | ||
|
@@ -1206,14 +1214,15 @@ fn check_matcher_core<'tt>( | |
)); | ||
} | ||
} | ||
err.emit(); | ||
errored = Err(err.emit()); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
last | ||
errored?; | ||
Ok(last) | ||
} | ||
|
||
fn token_can_be_followed_by_any(tok: &mbe::TokenTree) -> bool { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-existing, but updating
valid
here is pointless when it's immediately followed byreturn
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which means
check_lhs_nt_follows
doesn't need to return abool
.It would be good to know how/why the driver will abort after parsing/expansion, bonus points if you understand that and can augment this comment appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this return is in a closure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍