Skip to content
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

Merged
merged 6 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
39 changes: 24 additions & 15 deletions compiler/rustc_expand/src/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Contributor

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 by return.

Copy link
Contributor

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 a bool.

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.

Copy link
Contributor Author

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

return tt;
}
sess.dcx().span_bug(def.span, "wrong-structured lhs")
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can drop the trailing ?; and the Ok(()).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function returns Result<()> while the _core function returns an ok value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Ok(())
}

fn has_compile_error_macro(rhs: &mbe::TokenTree) -> bool {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
30 changes: 13 additions & 17 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.map(|vars| self.resolve_vars_if_possible(vars)),
);

self.report_arg_errors(
self.set_tainted_by_errors(self.report_arg_errors(
compatibility_diagonal,
formal_and_expected_inputs,
provided_args,
Expand All @@ -433,7 +433,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn_def_id,
call_span,
call_expr,
);
));
}
}

Expand All @@ -447,7 +447,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn_def_id: Option<DefId>,
call_span: Span,
call_expr: &'tcx hir::Expr<'tcx>,
) {
) -> ErrorGuaranteed {
// Next, let's construct the error
let (error_span, full_call_span, call_name, is_method) = match &call_expr.kind {
hir::ExprKind::Call(
Expand Down Expand Up @@ -486,10 +486,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

let tcx = self.tcx;
// FIXME: taint after emitting errors and pass through an `ErrorGuaranteed`
self.set_tainted_by_errors(
tcx.dcx().span_delayed_bug(call_span, "no errors reported for args"),
);

// Get the argument span in the context of the call span so that
// suggestions and labels are (more) correct when an arg is a
Expand Down Expand Up @@ -696,8 +692,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(mismatch_idx),
is_method,
);
err.emit();
return;
return err.emit();
}
}
}
Expand All @@ -721,11 +716,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if cfg!(debug_assertions) {
span_bug!(error_span, "expected errors from argument matrix");
} else {
tcx.dcx().emit_err(errors::ArgMismatchIndeterminate { span: error_span });
return tcx.dcx().emit_err(errors::ArgMismatchIndeterminate { span: error_span });
}
return;
}

let mut reported = None;
errors.retain(|error| {
let Error::Invalid(provided_idx, expected_idx, Compatibility::Incompatible(Some(e))) =
error
Expand All @@ -736,16 +731,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let trace =
mk_trace(provided_span, formal_and_expected_inputs[*expected_idx], provided_ty);
if !matches!(trace.cause.as_failure_code(*e), FailureCode::Error0308) {
self.err_ctxt().report_and_explain_type_error(trace, *e).emit();
reported = Some(self.err_ctxt().report_and_explain_type_error(trace, *e).emit());
return false;
}
true
});

// We're done if we found errors, but we already emitted them.
if errors.is_empty() {
return;
if let Some(reported) = reported {
assert!(errors.is_empty());
return reported;
}
assert!(!errors.is_empty());

// Okay, now that we've emitted the special errors separately, we
// are only left missing/extra/swapped and mismatched arguments, both
Expand Down Expand Up @@ -802,8 +799,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(expected_idx.as_usize()),
is_method,
);
err.emit();
return;
return err.emit();
}

let mut err = if formal_and_expected_inputs.len() == provided_args.len() {
Expand Down Expand Up @@ -1251,7 +1247,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}

err.emit();
err.emit()
}

fn suggest_ptr_null_mut(
Expand Down
13 changes: 0 additions & 13 deletions compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,6 @@ pub struct FnCtxt<'a, 'tcx> {
/// eventually).
pub(super) param_env: ty::ParamEnv<'tcx>,

/// Number of errors that had been reported when we started
/// checking this function. On exit, if we find that *more* errors
/// have been reported, we will skip regionck and other work that
/// expects the types within the function to be consistent.
// FIXME(matthewjasper) This should not exist, and it's not correct
// if type checking is run in parallel.
err_count_on_creation: usize,

/// If `Some`, this stores coercion information for returned
/// expressions. If `None`, this is in a context where return is
/// inappropriate, such as a const expression.
Expand Down Expand Up @@ -126,7 +118,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
FnCtxt {
body_id,
param_env,
err_count_on_creation: inh.tcx.dcx().err_count(),
ret_coercion: None,
ret_coercion_span: Cell::new(None),
coroutine_types: None,
Expand Down Expand Up @@ -195,10 +186,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}),
}
}

pub fn errors_reported_since_creation(&self) -> bool {
self.dcx().err_count() > self.err_count_on_creation
}
}

impl<'a, 'tcx> Deref for FnCtxt<'a, 'tcx> {
Expand Down
Loading