Skip to content

Commit 29fa927

Browse files
committed
Tweak error counting.
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.
1 parent 4783dcc commit 29fa927

File tree

14 files changed

+35
-32
lines changed

14 files changed

+35
-32
lines changed

Diff for: compiler/rustc_builtin_macros/src/format.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ fn make_format_args(
529529

530530
// Only check for unused named argument names if there are no other errors to avoid causing
531531
// too much noise in output errors, such as when a named argument is entirely unused.
532-
if invalid_refs.is_empty() && ecx.dcx().err_count() == 0 {
532+
if invalid_refs.is_empty() && ecx.dcx().has_errors().is_none() {
533533
for &(index, span, used_as) in &numeric_refences_to_named_arg {
534534
let (position_sp_to_replace, position_sp_for_msg) = match used_as {
535535
Placeholder(pspan) => (span, pspan),

Diff for: compiler/rustc_errors/src/lib.rs

+11-15
Original file line numberDiff line numberDiff line change
@@ -931,42 +931,38 @@ impl DiagCtxt {
931931
self.struct_bug(msg).emit()
932932
}
933933

934+
/// This excludes lint errors and delayed bugs.
934935
#[inline]
935936
pub fn err_count(&self) -> usize {
936937
self.inner.borrow().err_count
937938
}
938939

940+
/// This excludes lint errors and delayed bugs.
939941
pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
940942
self.inner.borrow().has_errors().then(|| {
941943
#[allow(deprecated)]
942944
ErrorGuaranteed::unchecked_claim_error_was_emitted()
943945
})
944946
}
945947

948+
/// This excludes delayed bugs. Unless absolutely necessary, prefer
949+
/// `has_errors` to this method.
946950
pub fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> {
947951
let inner = self.inner.borrow();
948-
let has_errors_or_lint_errors = inner.has_errors() || inner.lint_err_count > 0;
949-
has_errors_or_lint_errors.then(|| {
952+
let result = inner.has_errors() || inner.lint_err_count > 0;
953+
result.then(|| {
950954
#[allow(deprecated)]
951955
ErrorGuaranteed::unchecked_claim_error_was_emitted()
952956
})
953957
}
954958

955-
pub fn has_errors_or_span_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
959+
/// Unless absolutely necessary, prefer `has_errors` or
960+
/// `has_errors_or_lint_errors` to this method.
961+
pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
956962
let inner = self.inner.borrow();
957-
let has_errors_or_span_delayed_bugs =
958-
inner.has_errors() || !inner.span_delayed_bugs.is_empty();
959-
has_errors_or_span_delayed_bugs.then(|| {
960-
#[allow(deprecated)]
961-
ErrorGuaranteed::unchecked_claim_error_was_emitted()
962-
})
963-
}
964-
965-
pub fn is_compilation_going_to_fail(&self) -> Option<ErrorGuaranteed> {
966-
let inner = self.inner.borrow();
967-
let will_fail =
963+
let result =
968964
inner.has_errors() || inner.lint_err_count > 0 || !inner.span_delayed_bugs.is_empty();
969-
will_fail.then(|| {
965+
result.then(|| {
970966
#[allow(deprecated)]
971967
ErrorGuaranteed::unchecked_claim_error_was_emitted()
972968
})

Diff for: compiler/rustc_hir_analysis/src/check/wfcheck.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ where
117117
let errors = wfcx.select_all_or_error();
118118
if !errors.is_empty() {
119119
let err = infcx.err_ctxt().report_fulfillment_errors(errors);
120-
if tcx.dcx().err_count() > 0 {
120+
if tcx.dcx().has_errors().is_some() {
121121
return Err(err);
122122
} else {
123123
// HACK(oli-obk): tests/ui/specialization/min_specialization/specialize_on_type_error.rs

Diff for: compiler/rustc_incremental/src/persist/fs.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ pub fn finalize_session_directory(sess: &Session, svh: Option<Svh>) {
312312

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

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

Diff for: compiler/rustc_incremental/src/persist/save.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ pub fn save_dep_graph(tcx: TyCtxt<'_>) {
3131
if sess.opts.incremental.is_none() {
3232
return;
3333
}
34-
// This is going to be deleted in finalize_session_directory, so let's not create it
35-
if let Some(_) = sess.dcx().has_errors_or_span_delayed_bugs() {
34+
// This is going to be deleted in finalize_session_directory, so let's not create it.
35+
if sess.dcx().has_errors_or_lint_errors_or_delayed_bugs().is_some() {
3636
return;
3737
}
3838

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

Diff for: compiler/rustc_infer/src/infer/error_reporting/mod.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,9 @@ fn escape_literal(s: &str) -> String {
117117
/// field is only populated during an in-progress typeck.
118118
/// Get an instance by calling `InferCtxt::err_ctxt` or `FnCtxt::err_ctxt`.
119119
///
120-
/// You must only create this if you intend to actually emit an error.
121-
/// This provides a lot of utility methods which should not be used
122-
/// during the happy path.
120+
/// You must only create this if you intend to actually emit an error (or
121+
/// perhaps a warning, though preferably not.) It provides a lot of utility
122+
/// methods which should not be used during the happy path.
123123
pub struct TypeErrCtxt<'a, 'tcx> {
124124
pub infcx: &'a InferCtxt<'tcx>,
125125
pub typeck_results: Option<std::cell::Ref<'a, ty::TypeckResults<'tcx>>>,
@@ -133,9 +133,10 @@ pub struct TypeErrCtxt<'a, 'tcx> {
133133

134134
impl Drop for TypeErrCtxt<'_, '_> {
135135
fn drop(&mut self) {
136-
if let Some(_) = self.dcx().has_errors_or_span_delayed_bugs() {
137-
// ok, emitted an error.
136+
if self.dcx().has_errors().is_some() {
137+
// Ok, emitted an error.
138138
} else {
139+
// Didn't emit an error; maybe it was created but not yet emitted.
139140
self.infcx
140141
.tcx
141142
.sess

Diff for: compiler/rustc_middle/src/ty/visit.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,13 @@ pub trait TypeVisitableExt<'tcx>: TypeVisitable<TyCtxt<'tcx>> {
5555
}
5656
fn error_reported(&self) -> Result<(), ErrorGuaranteed> {
5757
if self.references_error() {
58-
if let Some(reported) = ty::tls::with(|tcx| tcx.dcx().is_compilation_going_to_fail()) {
58+
// We must include lint errors and span delayed bugs here.
59+
if let Some(reported) =
60+
ty::tls::with(|tcx| tcx.dcx().has_errors_or_lint_errors_or_delayed_bugs())
61+
{
5962
Err(reported)
6063
} else {
61-
bug!("expect tcx.sess.is_compilation_going_to_fail return `Some`");
64+
bug!("expected some kind of error in `error_reported`");
6265
}
6366
} else {
6467
Ok(())

Diff for: compiler/rustc_query_system/src/dep_graph/graph.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -818,7 +818,7 @@ impl<D: Deps> DepGraphData<D> {
818818
None => {}
819819
}
820820

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

Diff for: compiler/rustc_session/src/session.rs

+1
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ impl Session {
323323
}
324324

325325
pub fn compile_status(&self) -> Result<(), ErrorGuaranteed> {
326+
// We must include lint errors here.
326327
if let Some(reported) = self.dcx().has_errors_or_lint_errors() {
327328
let _ = self.dcx().emit_stashed_diagnostics();
328329
Err(reported)

Diff for: src/librustdoc/core.rs

+1
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,7 @@ pub(crate) fn run_global_ctxt(
449449

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

452+
// We must include lint errors here.
452453
if tcx.dcx().has_errors_or_lint_errors().is_some() {
453454
rustc_errors::FatalError.raise();
454455
}

Diff for: src/librustdoc/doctest.rs

+1
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
150150

151151
collector
152152
});
153+
// We must include lint errors here.
153154
if compiler.sess.dcx().has_errors_or_lint_errors().is_some() {
154155
FatalError.raise();
155156
}

Diff for: src/librustdoc/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,7 @@ fn main_args(
796796

797797
compiler.enter(|queries| {
798798
let mut gcx = abort_on_err(queries.global_ctxt(), sess);
799-
if sess.dcx().has_errors_or_lint_errors().is_some() {
799+
if sess.dcx().has_errors().is_some() {
800800
sess.dcx().fatal("Compilation failed, aborting rustdoc");
801801
}
802802

Diff for: src/librustdoc/passes/collect_trait_impls.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) ->
2222
let tcx = cx.tcx;
2323
// We need to check if there are errors before running this pass because it would crash when
2424
// we try to get auto and blanket implementations.
25-
if tcx.dcx().has_errors_or_lint_errors().is_some() {
25+
if tcx.dcx().has_errors().is_some() {
2626
return krate;
2727
}
2828

Diff for: src/librustdoc/scrape_examples.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ pub(crate) fn run(
311311

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

0 commit comments

Comments
 (0)