Skip to content

Commit

Permalink
Tweak error counting.
Browse files Browse the repository at this point in the history
We have several methods indicating the presence of errors, lint errors,
and delayed bugs. I find it frustrating that it's very unclear which one
you should use in any particular spot. This commit attempts to instill a
basic principle of "use the least general one possible", because that
reflects reality in practice -- `has_errors` is the least general one
and has by far the most uses (esp. via `abort_if_errors`).

Specifics:
- Add some comments giving some usage guidelines.
- Prefer `has_errors` to comparing `err_count` to zero.
- Remove `has_errors_or_span_delayed_bugs` because it's a weird one: in
  the cases where we need to count delayed bugs, we should really be
  counting lint errors as well.
- Rename `is_compilation_going_to_fail` as
  `has_errors_or_lint_errors_or_span_delayed_bugs`, for consistency with
  `has_errors` and `has_errors_or_lint_errors`.
- Change a few other `has_errors_or_lint_errors` calls to `has_errors`,
  as per the "least general" principle.

This didn't turn out to be as neat as I hoped when I started, but I
think it's still an improvement.
  • Loading branch information
nnethercote committed Jan 21, 2024
1 parent 807c868 commit 1f9fa23
Show file tree
Hide file tree
Showing 14 changed files with 35 additions and 32 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ fn make_format_args(

// Only check for unused named argument names if there are no other errors to avoid causing
// too much noise in output errors, such as when a named argument is entirely unused.
if invalid_refs.is_empty() && ecx.dcx().err_count() == 0 {
if invalid_refs.is_empty() && ecx.dcx().has_errors().is_none() {
for &(index, span, used_as) in &numeric_refences_to_named_arg {
let (position_sp_to_replace, position_sp_for_msg) = match used_as {
Placeholder(pspan) => (span, pspan),
Expand Down
26 changes: 11 additions & 15 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,42 +927,38 @@ impl DiagCtxt {
self.struct_bug(msg).emit()
}

/// This excludes lint errors and delayed bugs.
#[inline]
pub fn err_count(&self) -> usize {
self.inner.borrow().err_count
}

/// This excludes lint errors and delayed bugs.
pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
self.inner.borrow().has_errors().then(|| {
#[allow(deprecated)]
ErrorGuaranteed::unchecked_claim_error_was_emitted()
})
}

/// This excludes delayed bugs. Unless absolutely necessary, prefer
/// `has_errors` to this method.
pub fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> {
let inner = self.inner.borrow();
let has_errors_or_lint_errors = inner.has_errors() || inner.lint_err_count > 0;
has_errors_or_lint_errors.then(|| {
let result = inner.has_errors() || inner.lint_err_count > 0;
result.then(|| {
#[allow(deprecated)]
ErrorGuaranteed::unchecked_claim_error_was_emitted()
})
}

pub fn has_errors_or_span_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
/// Unless absolutely necessary, prefer `has_errors` or
/// `has_errors_or_lint_errors` to this method.
pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
let inner = self.inner.borrow();
let has_errors_or_span_delayed_bugs =
inner.has_errors() || !inner.span_delayed_bugs.is_empty();
has_errors_or_span_delayed_bugs.then(|| {
#[allow(deprecated)]
ErrorGuaranteed::unchecked_claim_error_was_emitted()
})
}

pub fn is_compilation_going_to_fail(&self) -> Option<ErrorGuaranteed> {
let inner = self.inner.borrow();
let will_fail =
let result =
inner.has_errors() || inner.lint_err_count > 0 || !inner.span_delayed_bugs.is_empty();
will_fail.then(|| {
result.then(|| {
#[allow(deprecated)]
ErrorGuaranteed::unchecked_claim_error_was_emitted()
})
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ where
let errors = wfcx.select_all_or_error();
if !errors.is_empty() {
let err = infcx.err_ctxt().report_fulfillment_errors(errors);
if tcx.dcx().err_count() > 0 {
if tcx.dcx().has_errors().is_some() {
return Err(err);
} else {
// HACK(oli-obk): tests/ui/specialization/min_specialization/specialize_on_type_error.rs
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_incremental/src/persist/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ pub fn finalize_session_directory(sess: &Session, svh: Option<Svh>) {

let incr_comp_session_dir: PathBuf = sess.incr_comp_session_dir().clone();

if let Some(_) = sess.dcx().has_errors_or_span_delayed_bugs() {
if sess.dcx().has_errors_or_lint_errors_or_delayed_bugs().is_some() {
// If there have been any errors during compilation, we don't want to
// publish this session directory. Rather, we'll just delete it.

Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_incremental/src/persist/save.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ pub fn save_dep_graph(tcx: TyCtxt<'_>) {
if sess.opts.incremental.is_none() {
return;
}
// This is going to be deleted in finalize_session_directory, so let's not create it
if let Some(_) = sess.dcx().has_errors_or_span_delayed_bugs() {
// This is going to be deleted in finalize_session_directory, so let's not create it.
if sess.dcx().has_errors_or_lint_errors_or_delayed_bugs().is_some() {
return;
}

Expand Down Expand Up @@ -87,7 +87,7 @@ pub fn save_work_product_index(
return;
}
// This is going to be deleted in finalize_session_directory, so let's not create it
if let Some(_) = sess.dcx().has_errors_or_span_delayed_bugs() {
if sess.dcx().has_errors_or_lint_errors().is_some() {
return;
}

Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ fn escape_literal(s: &str) -> String {
/// field is only populated during an in-progress typeck.
/// Get an instance by calling `InferCtxt::err_ctxt` or `FnCtxt::err_ctxt`.
///
/// You must only create this if you intend to actually emit an error.
/// This provides a lot of utility methods which should not be used
/// during the happy path.
/// You must only create this if you intend to actually emit an error (or
/// perhaps a warning, though preferably not.) It provides a lot of utility
/// methods which should not be used during the happy path.
pub struct TypeErrCtxt<'a, 'tcx> {
pub infcx: &'a InferCtxt<'tcx>,
pub typeck_results: Option<std::cell::Ref<'a, ty::TypeckResults<'tcx>>>,
Expand All @@ -133,9 +133,10 @@ pub struct TypeErrCtxt<'a, 'tcx> {

impl Drop for TypeErrCtxt<'_, '_> {
fn drop(&mut self) {
if let Some(_) = self.dcx().has_errors_or_span_delayed_bugs() {
// ok, emitted an error.
if self.dcx().has_errors().is_some() {
// Ok, emitted an error.
} else {
// Didn't emit an error; maybe it was created but not yet emitted.
self.infcx
.tcx
.sess
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_middle/src/ty/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,13 @@ pub trait TypeVisitableExt<'tcx>: TypeVisitable<TyCtxt<'tcx>> {
}
fn error_reported(&self) -> Result<(), ErrorGuaranteed> {
if self.references_error() {
if let Some(reported) = ty::tls::with(|tcx| tcx.dcx().is_compilation_going_to_fail()) {
// We must include lint errors and span delayed bugs here.
if let Some(reported) =
ty::tls::with(|tcx| tcx.dcx().has_errors_or_lint_errors_or_delayed_bugs())
{
Err(reported)
} else {
bug!("expect tcx.sess.is_compilation_going_to_fail return `Some`");
bug!("expected some kind of error in `error_reported`");
}
} else {
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_query_system/src/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ impl<D: Deps> DepGraphData<D> {
None => {}
}

if let None = qcx.dep_context().sess().dcx().has_errors_or_span_delayed_bugs() {
if let None = qcx.dep_context().sess().dcx().has_errors_or_lint_errors_or_delayed_bugs() {
panic!("try_mark_previous_green() - Forcing the DepNode should have set its color")
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ impl Session {
}

pub fn compile_status(&self) -> Result<(), ErrorGuaranteed> {
// We must include lint errors here.
if let Some(reported) = self.dcx().has_errors_or_lint_errors() {
let _ = self.dcx().emit_stashed_diagnostics();
Err(reported)
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ pub(crate) fn run_global_ctxt(

tcx.sess.time("check_lint_expectations", || tcx.check_expectations(Some(sym::rustdoc)));

// We must include lint errors here.
if tcx.dcx().has_errors_or_lint_errors().is_some() {
rustc_errors::FatalError.raise();
}
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/doctest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {

collector
});
// We must include lint errors here.
if compiler.sess.dcx().has_errors_or_lint_errors().is_some() {
FatalError.raise();
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ fn main_args(

compiler.enter(|queries| {
let mut gcx = abort_on_err(queries.global_ctxt(), sess);
if sess.dcx().has_errors_or_lint_errors().is_some() {
if sess.dcx().has_errors().is_some() {
sess.dcx().fatal("Compilation failed, aborting rustdoc");
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/collect_trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) ->
let tcx = cx.tcx;
// We need to check if there are errors before running this pass because it would crash when
// we try to get auto and blanket implementations.
if tcx.dcx().has_errors_or_lint_errors().is_some() {
if tcx.dcx().has_errors().is_some() {
return krate;
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/scrape_examples.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ pub(crate) fn run(

// The visitor might have found a type error, which we need to
// promote to a fatal error
if tcx.dcx().has_errors_or_lint_errors().is_some() {
if tcx.dcx().has_errors().is_some() {
return Err(String::from("Compilation failed, aborting rustdoc"));
}

Expand Down

0 comments on commit 1f9fa23

Please sign in to comment.