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.
This commit is contained in:
parent
807c8687de
commit
1f9fa2305a
@ -529,7 +529,7 @@ enum ArgRef<'a> {
|
|||||||
|
|
||||||
// Only check for unused named argument names if there are no other errors to avoid causing
|
// 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.
|
// 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 {
|
for &(index, span, used_as) in &numeric_refences_to_named_arg {
|
||||||
let (position_sp_to_replace, position_sp_for_msg) = match used_as {
|
let (position_sp_to_replace, position_sp_for_msg) = match used_as {
|
||||||
Placeholder(pspan) => (span, pspan),
|
Placeholder(pspan) => (span, pspan),
|
||||||
|
@ -927,11 +927,13 @@ pub fn bug(&self, msg: impl Into<DiagnosticMessage>) -> ! {
|
|||||||
self.struct_bug(msg).emit()
|
self.struct_bug(msg).emit()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// This excludes lint errors and delayed bugs.
|
||||||
#[inline]
|
#[inline]
|
||||||
pub fn err_count(&self) -> usize {
|
pub fn err_count(&self) -> usize {
|
||||||
self.inner.borrow().err_count
|
self.inner.borrow().err_count
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// This excludes lint errors and delayed bugs.
|
||||||
pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
|
pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
|
||||||
self.inner.borrow().has_errors().then(|| {
|
self.inner.borrow().has_errors().then(|| {
|
||||||
#[allow(deprecated)]
|
#[allow(deprecated)]
|
||||||
@ -939,30 +941,24 @@ pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// This excludes delayed bugs. Unless absolutely necessary, prefer
|
||||||
|
/// `has_errors` to this method.
|
||||||
pub fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> {
|
pub fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> {
|
||||||
let inner = self.inner.borrow();
|
let inner = self.inner.borrow();
|
||||||
let has_errors_or_lint_errors = inner.has_errors() || inner.lint_err_count > 0;
|
let result = inner.has_errors() || inner.lint_err_count > 0;
|
||||||
has_errors_or_lint_errors.then(|| {
|
result.then(|| {
|
||||||
#[allow(deprecated)]
|
#[allow(deprecated)]
|
||||||
ErrorGuaranteed::unchecked_claim_error_was_emitted()
|
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 inner = self.inner.borrow();
|
||||||
let has_errors_or_span_delayed_bugs =
|
let result =
|
||||||
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 =
|
|
||||||
inner.has_errors() || inner.lint_err_count > 0 || !inner.span_delayed_bugs.is_empty();
|
inner.has_errors() || inner.lint_err_count > 0 || !inner.span_delayed_bugs.is_empty();
|
||||||
will_fail.then(|| {
|
result.then(|| {
|
||||||
#[allow(deprecated)]
|
#[allow(deprecated)]
|
||||||
ErrorGuaranteed::unchecked_claim_error_was_emitted()
|
ErrorGuaranteed::unchecked_claim_error_was_emitted()
|
||||||
})
|
})
|
||||||
|
@ -116,7 +116,7 @@ pub(super) fn enter_wf_checking_ctxt<'tcx, F>(
|
|||||||
let errors = wfcx.select_all_or_error();
|
let errors = wfcx.select_all_or_error();
|
||||||
if !errors.is_empty() {
|
if !errors.is_empty() {
|
||||||
let err = infcx.err_ctxt().report_fulfillment_errors(errors);
|
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);
|
return Err(err);
|
||||||
} else {
|
} else {
|
||||||
// HACK(oli-obk): tests/ui/specialization/min_specialization/specialize_on_type_error.rs
|
// HACK(oli-obk): tests/ui/specialization/min_specialization/specialize_on_type_error.rs
|
||||||
|
@ -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();
|
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
|
// If there have been any errors during compilation, we don't want to
|
||||||
// publish this session directory. Rather, we'll just delete it.
|
// publish this session directory. Rather, we'll just delete it.
|
||||||
|
|
||||||
|
@ -31,8 +31,8 @@ pub fn save_dep_graph(tcx: TyCtxt<'_>) {
|
|||||||
if sess.opts.incremental.is_none() {
|
if sess.opts.incremental.is_none() {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
// This is going to be deleted in finalize_session_directory, so let's not create it
|
// 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_or_delayed_bugs().is_some() {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -87,7 +87,7 @@ pub fn save_work_product_index(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
// This is going to be deleted in finalize_session_directory, so let's not create it
|
// 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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -117,9 +117,9 @@ fn escape_literal(s: &str) -> String {
|
|||||||
/// field is only populated during an in-progress typeck.
|
/// field is only populated during an in-progress typeck.
|
||||||
/// Get an instance by calling `InferCtxt::err_ctxt` or `FnCtxt::err_ctxt`.
|
/// 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.
|
/// You must only create this if you intend to actually emit an error (or
|
||||||
/// This provides a lot of utility methods which should not be used
|
/// perhaps a warning, though preferably not.) It provides a lot of utility
|
||||||
/// during the happy path.
|
/// methods which should not be used during the happy path.
|
||||||
pub struct TypeErrCtxt<'a, 'tcx> {
|
pub struct TypeErrCtxt<'a, 'tcx> {
|
||||||
pub infcx: &'a InferCtxt<'tcx>,
|
pub infcx: &'a InferCtxt<'tcx>,
|
||||||
pub typeck_results: Option<std::cell::Ref<'a, ty::TypeckResults<'tcx>>>,
|
pub typeck_results: Option<std::cell::Ref<'a, ty::TypeckResults<'tcx>>>,
|
||||||
@ -133,9 +133,10 @@ pub struct TypeErrCtxt<'a, 'tcx> {
|
|||||||
|
|
||||||
impl Drop for TypeErrCtxt<'_, '_> {
|
impl Drop for TypeErrCtxt<'_, '_> {
|
||||||
fn drop(&mut self) {
|
fn drop(&mut self) {
|
||||||
if let Some(_) = self.dcx().has_errors_or_span_delayed_bugs() {
|
if self.dcx().has_errors().is_some() {
|
||||||
// ok, emitted an error.
|
// Ok, emitted an error.
|
||||||
} else {
|
} else {
|
||||||
|
// Didn't emit an error; maybe it was created but not yet emitted.
|
||||||
self.infcx
|
self.infcx
|
||||||
.tcx
|
.tcx
|
||||||
.sess
|
.sess
|
||||||
|
@ -55,10 +55,13 @@ fn references_error(&self) -> bool {
|
|||||||
}
|
}
|
||||||
fn error_reported(&self) -> Result<(), ErrorGuaranteed> {
|
fn error_reported(&self) -> Result<(), ErrorGuaranteed> {
|
||||||
if self.references_error() {
|
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)
|
Err(reported)
|
||||||
} else {
|
} else {
|
||||||
bug!("expect tcx.sess.is_compilation_going_to_fail return `Some`");
|
bug!("expected some kind of error in `error_reported`");
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
Ok(())
|
Ok(())
|
||||||
|
@ -818,7 +818,7 @@ fn try_mark_parent_green<Qcx: QueryContext<Deps = D>>(
|
|||||||
None => {}
|
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")
|
panic!("try_mark_previous_green() - Forcing the DepNode should have set its color")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -323,6 +323,7 @@ pub fn create_feature_err<'a>(
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn compile_status(&self) -> Result<(), ErrorGuaranteed> {
|
pub fn compile_status(&self) -> Result<(), ErrorGuaranteed> {
|
||||||
|
// We must include lint errors here.
|
||||||
if let Some(reported) = self.dcx().has_errors_or_lint_errors() {
|
if let Some(reported) = self.dcx().has_errors_or_lint_errors() {
|
||||||
let _ = self.dcx().emit_stashed_diagnostics();
|
let _ = self.dcx().emit_stashed_diagnostics();
|
||||||
Err(reported)
|
Err(reported)
|
||||||
|
@ -449,6 +449,7 @@ fn report_deprecated_attr(name: &str, dcx: &rustc_errors::DiagCtxt, sp: Span) {
|
|||||||
|
|
||||||
tcx.sess.time("check_lint_expectations", || tcx.check_expectations(Some(sym::rustdoc)));
|
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() {
|
if tcx.dcx().has_errors_or_lint_errors().is_some() {
|
||||||
rustc_errors::FatalError.raise();
|
rustc_errors::FatalError.raise();
|
||||||
}
|
}
|
||||||
|
@ -150,6 +150,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
|
|||||||
|
|
||||||
collector
|
collector
|
||||||
});
|
});
|
||||||
|
// We must include lint errors here.
|
||||||
if compiler.sess.dcx().has_errors_or_lint_errors().is_some() {
|
if compiler.sess.dcx().has_errors_or_lint_errors().is_some() {
|
||||||
FatalError.raise();
|
FatalError.raise();
|
||||||
}
|
}
|
||||||
|
@ -796,7 +796,7 @@ fn main_args(
|
|||||||
|
|
||||||
compiler.enter(|queries| {
|
compiler.enter(|queries| {
|
||||||
let mut gcx = abort_on_err(queries.global_ctxt(), sess);
|
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");
|
sess.dcx().fatal("Compilation failed, aborting rustdoc");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -22,7 +22,7 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) ->
|
|||||||
let tcx = cx.tcx;
|
let tcx = cx.tcx;
|
||||||
// We need to check if there are errors before running this pass because it would crash when
|
// 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.
|
// 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;
|
return krate;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -311,7 +311,7 @@ pub(crate) fn run(
|
|||||||
|
|
||||||
// The visitor might have found a type error, which we need to
|
// The visitor might have found a type error, which we need to
|
||||||
// promote to a fatal error
|
// 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"));
|
return Err(String::from("Compilation failed, aborting rustdoc"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user