Skip to content

Commit

Permalink
Auto merge of rust-lang#120226 - matthiaskrgr:rollup-9xwx0si, r=matth…
Browse files Browse the repository at this point in the history
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#118714 ( Explanation that fields are being used when deriving `(Partial)Ord` on enums)
 - rust-lang#119710 (Improve `let_underscore_lock`)
 - rust-lang#119726 (Tweak Library Integer Division Docs)
 - rust-lang#119746 (rustdoc: hide modals when resizing the sidebar)
 - rust-lang#119986 (Fix error counting)
 - rust-lang#120194 (Shorten `#[must_use]` Diagnostic Message for `Option::is_none`)
 - rust-lang#120200 (Correct the anchor of an URL in an error message)
 - rust-lang#120203 (Replace `#!/bin/bash` with `#!/usr/bin/env bash` in rust-installer tests)
 - rust-lang#120212 (Give nnethercote more reviews)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jan 22, 2024
2 parents 6fff796 + 610f13d commit 366d112
Show file tree
Hide file tree
Showing 30 changed files with 267 additions and 124 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
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ codegen_ssa_unsupported_arch = unsupported arch `{$arch}` for os `{$os}`
codegen_ssa_unsupported_link_self_contained = option `-C link-self-contained` is not supported on this target
codegen_ssa_use_cargo_directive = use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname)
codegen_ssa_use_cargo_directive = use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-link-lib)
codegen_ssa_version_script_write_failure = failed to write version script: {$error}
Expand Down
40 changes: 18 additions & 22 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,16 +421,16 @@ pub struct DiagCtxt {
struct DiagCtxtInner {
flags: DiagCtxtFlags,

/// The number of lint errors that have been emitted.
/// The number of lint errors that have been emitted, including duplicates.
lint_err_count: usize,
/// The number of errors that have been emitted, including duplicates.
///
/// This is not necessarily the count that's reported to the user once
/// compilation ends.
/// The number of non-lint errors that have been emitted, including duplicates.
err_count: usize,

/// The error count shown to the user at the end.
deduplicated_err_count: usize,
/// The warning count, used for a recap upon finishing
/// The warning count shown to the user at the end.
deduplicated_warn_count: usize,

/// Has this diagnostic context printed any diagnostics? (I.e. has
/// `self.emitter.emit_diagnostic()` been called?
has_printed: bool,
Expand Down 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(|| {
#[allow(deprecated)]
ErrorGuaranteed::unchecked_claim_error_was_emitted()
})
}

pub fn has_errors_or_span_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(|| {
let result = inner.has_errors() || inner.lint_err_count > 0;
result.then(|| {
#[allow(deprecated)]
ErrorGuaranteed::unchecked_claim_error_was_emitted()
})
}

pub fn is_compilation_going_to_fail(&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 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 Expand Up @@ -1162,7 +1158,7 @@ impl DiagCtxt {
let mut inner = self.inner.borrow_mut();

if loud && lint_level.is_error() {
inner.err_count += 1;
inner.lint_err_count += 1;
inner.panic_if_treat_err_as_bug();
}

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
3 changes: 3 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,9 @@ lint_multiple_supertrait_upcastable = `{$ident}` is object-safe and has multiple
lint_node_source = `forbid` level set here
.note = {$reason}
lint_non_binding_let_multi_drop_fn =
consider immediately dropping the value using `drop(..)` after the `let` statement
lint_non_binding_let_multi_suggestion =
consider immediately dropping the value
Expand Down
59 changes: 39 additions & 20 deletions compiler/rustc_lint/src/let_underscore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
use rustc_errors::MultiSpan;
use rustc_hir as hir;
use rustc_middle::ty;
use rustc_span::Symbol;
use rustc_span::{sym, Symbol};

declare_lint! {
/// The `let_underscore_drop` lint checks for statements which don't bind
Expand Down Expand Up @@ -105,51 +105,70 @@ const SYNC_GUARD_SYMBOLS: [Symbol; 3] = [

impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
fn check_local(&mut self, cx: &LateContext<'_>, local: &hir::Local<'_>) {
if !matches!(local.pat.kind, hir::PatKind::Wild) {
return;
}

if matches!(local.source, rustc_hir::LocalSource::AsyncFn) {
return;
}
if let Some(init) = local.init {
let init_ty = cx.typeck_results().expr_ty(init);

let mut top_level = true;

// We recursively walk through all patterns, so that we can catch cases where the lock is nested in a pattern.
// For the basic `let_underscore_drop` lint, we only look at the top level, since there are many legitimate reasons
// to bind a sub-pattern to an `_`, if we're only interested in the rest.
// But with locks, we prefer having the chance of "false positives" over missing cases, since the effects can be
// quite catastrophic.
local.pat.walk_always(|pat| {
let is_top_level = top_level;
top_level = false;

if !matches!(pat.kind, hir::PatKind::Wild) {
return;
}

let ty = cx.typeck_results().pat_ty(pat);

// If the type has a trivial Drop implementation, then it doesn't
// matter that we drop the value immediately.
if !init_ty.needs_drop(cx.tcx, cx.param_env) {
if !ty.needs_drop(cx.tcx, cx.param_env) {
return;
}
let is_sync_lock = match init_ty.kind() {
// Lint for patterns like `mutex.lock()`, which returns `Result<MutexGuard, _>` as well.
let potential_lock_type = match ty.kind() {
ty::Adt(adt, args) if cx.tcx.is_diagnostic_item(sym::Result, adt.did()) => {
args.type_at(0)
}
_ => ty,
};
let is_sync_lock = match potential_lock_type.kind() {
ty::Adt(adt, _) => SYNC_GUARD_SYMBOLS
.iter()
.any(|guard_symbol| cx.tcx.is_diagnostic_item(*guard_symbol, adt.did())),
_ => false,
};

let can_use_init = is_top_level.then_some(local.init).flatten();

let sub = NonBindingLetSub {
suggestion: local.pat.span,
multi_suggestion_start: local.span.until(init.span),
multi_suggestion_end: init.span.shrink_to_hi(),
suggestion: pat.span,
// We can't suggest `drop()` when we're on the top level.
drop_fn_start_end: can_use_init
.map(|init| (local.span.until(init.span), init.span.shrink_to_hi())),
is_assign_desugar: matches!(local.source, rustc_hir::LocalSource::AssignDesugar(_)),
};
if is_sync_lock {
let mut span = MultiSpan::from_spans(vec![local.pat.span, init.span]);
let mut span = MultiSpan::from_span(pat.span);
span.push_span_label(
local.pat.span,
pat.span,
"this lock is not assigned to a binding and is immediately dropped".to_string(),
);
span.push_span_label(
init.span,
"this binding will immediately drop the value assigned to it".to_string(),
);
cx.emit_spanned_lint(LET_UNDERSCORE_LOCK, span, NonBindingLet::SyncLock { sub });
} else {
// Only emit let_underscore_drop for top-level `_` patterns.
} else if can_use_init.is_some() {
cx.emit_spanned_lint(
LET_UNDERSCORE_DROP,
local.span,
NonBindingLet::DropType { sub },
);
}
}
});
}
}
43 changes: 26 additions & 17 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -930,8 +930,7 @@ pub enum NonBindingLet {

pub struct NonBindingLetSub {
pub suggestion: Span,
pub multi_suggestion_start: Span,
pub multi_suggestion_end: Span,
pub drop_fn_start_end: Option<(Span, Span)>,
pub is_assign_desugar: bool,
}

Expand All @@ -940,21 +939,31 @@ impl AddToDiagnostic for NonBindingLetSub {
where
F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage,
{
let prefix = if self.is_assign_desugar { "let " } else { "" };
diag.span_suggestion_verbose(
self.suggestion,
fluent::lint_non_binding_let_suggestion,
format!("{prefix}_unused"),
Applicability::MachineApplicable,
);
diag.multipart_suggestion(
fluent::lint_non_binding_let_multi_suggestion,
vec![
(self.multi_suggestion_start, "drop(".to_string()),
(self.multi_suggestion_end, ")".to_string()),
],
Applicability::MachineApplicable,
);
let can_suggest_binding = self.drop_fn_start_end.is_some() || !self.is_assign_desugar;

if can_suggest_binding {
let prefix = if self.is_assign_desugar { "let " } else { "" };
diag.span_suggestion_verbose(
self.suggestion,
fluent::lint_non_binding_let_suggestion,
format!("{prefix}_unused"),
Applicability::MachineApplicable,
);
} else {
diag.span_help(self.suggestion, fluent::lint_non_binding_let_suggestion);
}
if let Some(drop_fn_start_end) = self.drop_fn_start_end {
diag.multipart_suggestion(
fluent::lint_non_binding_let_multi_suggestion,
vec![
(drop_fn_start_end.0, "drop(".to_string()),
(drop_fn_start_end.1, ")".to_string()),
],
Applicability::MachineApplicable,
);
} else {
diag.help(fluent::lint_non_binding_let_multi_drop_fn);
}
}
}

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
6 changes: 4 additions & 2 deletions library/core/src/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,8 @@ impl<T: Clone> Clone for Reverse<T> {
/// [lexicographic](https://en.wikipedia.org/wiki/Lexicographic_order) ordering
/// based on the top-to-bottom declaration order of the struct's members.
///
/// When `derive`d on enums, variants are ordered by their discriminants.
/// When `derive`d on enums, variants are ordered primarily by their discriminants.
/// Secondarily, they are ordered by their fields.
/// By default, the discriminant is smallest for variants at the top, and
/// largest for variants at the bottom. Here's an example:
///
Expand Down Expand Up @@ -963,7 +964,8 @@ pub macro Ord($item:item) {
/// [lexicographic](https://en.wikipedia.org/wiki/Lexicographic_order) ordering
/// based on the top-to-bottom declaration order of the struct's members.
///
/// When `derive`d on enums, variants are ordered by their discriminants.
/// When `derive`d on enums, variants are primarily ordered by their discriminants.
/// Secondarily, they are ordered by their fields.
/// By default, the discriminant is smallest for variants at the top, and
/// largest for variants at the bottom. Here's an example:
///
Expand Down
Loading

0 comments on commit 366d112

Please sign in to comment.