Skip to content

Commit

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

Rollup of 6 pull requests

Successful merges:

 - rust-lang#134455 (cleanup promoteds move check)
 - rust-lang#135421 (Make tidy warn on unrecognized directives)
 - rust-lang#135611 (Remove unnecessary assertion for reference error)
 - rust-lang#135620 (ci: improve github action name)
 - rust-lang#135639 (new solver: prefer trivial builtin impls)
 - rust-lang#135654 (add src/librustdoc and src/rustdoc-json-types to RUSTC_IF_UNCHANGED_ALLOWED_PATHS)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jan 18, 2025
2 parents bd62a45 + e873695 commit 8321f00
Show file tree
Hide file tree
Showing 17 changed files with 134 additions and 52 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ghcr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
# for PR jobs, because forks can't access secrets.
# That's why we use ghcr.io: it has no rate limit and it doesn't require authentication.

name: GHCR
name: GHCR image mirroring

on:
workflow_dispatch:
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4259,6 +4259,7 @@ dependencies = [
"rustc_serialize",
"rustc_type_ir",
"rustc_type_ir_macros",
"smallvec",
"tracing",
]

Expand Down
18 changes: 9 additions & 9 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,6 @@ fn do_mir_borrowck<'tcx>(
let location_table = PoloniusLocationTable::new(body);

let move_data = MoveData::gather_moves(body, tcx, |_| true);
let promoted_move_data = promoted
.iter_enumerated()
.map(|(idx, body)| (idx, MoveData::gather_moves(body, tcx, |_| true)));

let flow_inits = MaybeInitializedPlaces::new(tcx, body, &move_data)
.iterate_to_fixpoint(tcx, body, Some("borrowck"))
Expand Down Expand Up @@ -242,10 +239,14 @@ fn do_mir_borrowck<'tcx>(
false
};

for (idx, move_data) in promoted_move_data {
// While promoteds should mostly be correct by construction, we need to check them for
// invalid moves to detect moving out of arrays:`struct S; fn main() { &([S][0]); }`.
for promoted_body in &promoted {
use rustc_middle::mir::visit::Visitor;

let promoted_body = &promoted[idx];
// This assumes that we won't use some of the fields of the `promoted_mbcx`
// when detecting and reporting move errors. While it would be nice to move
// this check out of `MirBorrowckCtxt`, actually doing so is far from trivial.
let move_data = MoveData::gather_moves(promoted_body, tcx, |_| true);
let mut promoted_mbcx = MirBorrowckCtxt {
infcx: &infcx,
body: promoted_body,
Expand All @@ -270,9 +271,6 @@ fn do_mir_borrowck<'tcx>(
move_errors: Vec::new(),
diags_buffer,
};
MoveVisitor { ctxt: &mut promoted_mbcx }.visit_body(promoted_body);
promoted_mbcx.report_move_errors();

struct MoveVisitor<'a, 'b, 'infcx, 'tcx> {
ctxt: &'a mut MirBorrowckCtxt<'b, 'infcx, 'tcx>,
}
Expand All @@ -284,6 +282,8 @@ fn do_mir_borrowck<'tcx>(
}
}
}
MoveVisitor { ctxt: &mut promoted_mbcx }.visit_body(promoted_body);
promoted_mbcx.report_move_errors();
}

let mut mbcx = MirBorrowckCtxt {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1637,7 +1637,6 @@ fn check_type_alias_type_params_are_used<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalD
let ty = tcx.type_of(def_id).instantiate_identity();
if ty.references_error() {
// If there is already another error, do not emit an error for not using a type parameter.
assert!(tcx.dcx().has_errors().is_some());
return;
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_next_trait_solver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ rustc_macros = { path = "../rustc_macros", optional = true }
rustc_serialize = { path = "../rustc_serialize", optional = true }
rustc_type_ir = { path = "../rustc_type_ir", default-features = false }
rustc_type_ir_macros = { path = "../rustc_type_ir_macros" }
smallvec = "1.8.1"
tracing = "0.1"
# tidy-alphabetical-end

Expand Down
28 changes: 26 additions & 2 deletions compiler/rustc_next_trait_solver/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use rustc_type_ir::lang_items::TraitSolverLangItem;
use rustc_type_ir::solve::CanonicalResponse;
use rustc_type_ir::visit::TypeVisitableExt as _;
use rustc_type_ir::{self as ty, Interner, TraitPredicate, TypingMode, Upcast as _, elaborate};
use smallvec::SmallVec;
use tracing::{instrument, trace};

use crate::delegate::SolverDelegate;
Expand Down Expand Up @@ -225,7 +226,7 @@ where
}

ecx.probe_and_evaluate_goal_for_constituent_tys(
CandidateSource::BuiltinImpl(BuiltinImplSource::Misc),
CandidateSource::BuiltinImpl(BuiltinImplSource::Trivial),
goal,
structural_traits::instantiate_constituent_tys_for_sized_trait,
)
Expand Down Expand Up @@ -1194,7 +1195,30 @@ where
};
}

// FIXME: prefer trivial builtin impls
// We prefer trivial builtin candidates, i.e. builtin impls without any
// nested requirements, over all others. This is a fix for #53123 and
// prevents where-bounds from accidentally extending the lifetime of a
// variable.
if candidates
.iter()
.any(|c| matches!(c.source, CandidateSource::BuiltinImpl(BuiltinImplSource::Trivial)))
{
let trivial_builtin_impls: SmallVec<[_; 1]> = candidates
.iter()
.filter(|c| {
matches!(c.source, CandidateSource::BuiltinImpl(BuiltinImplSource::Trivial))
})
.map(|c| c.result)
.collect();
// There should only ever be a single trivial builtin candidate
// as they would otherwise overlap.
assert_eq!(trivial_builtin_impls.len(), 1);
return if let Some(response) = self.try_merge_responses(&trivial_builtin_impls) {
Ok((response, Some(TraitGoalProvenVia::Misc)))
} else {
Ok((self.bail_with_ambiguity(&trivial_builtin_impls), None))
};
}

// If there are non-global where-bounds, prefer where-bounds
// (including global ones) over everything else.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
Err(ErrorGuaranteed { .. }) => true,
}
}
ImplSource::Builtin(BuiltinImplSource::Misc, _) => {
ImplSource::Builtin(BuiltinImplSource::Misc | BuiltinImplSource::Trivial, _) => {
// While a builtin impl may be known to exist, the associated type may not yet
// be known. Any type with multiple potential associated types is therefore
// not eligible.
Expand Down Expand Up @@ -1296,7 +1296,7 @@ fn confirm_select_candidate<'cx, 'tcx>(
) -> Progress<'tcx> {
match impl_source {
ImplSource::UserDefined(data) => confirm_impl_candidate(selcx, obligation, data),
ImplSource::Builtin(BuiltinImplSource::Misc, data) => {
ImplSource::Builtin(BuiltinImplSource::Misc | BuiltinImplSource::Trivial, data) => {
let tcx = selcx.tcx();
let trait_def_id = obligation.predicate.trait_def_id(tcx);
if tcx.is_lang_item(trait_def_id, LangItem::Coroutine) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ fn resolve_associated_item<'tcx>(
})
}
}
traits::ImplSource::Builtin(BuiltinImplSource::Misc, _) => {
traits::ImplSource::Builtin(BuiltinImplSource::Misc | BuiltinImplSource::Trivial, _) => {
if tcx.is_lang_item(trait_ref.def_id, LangItem::Clone) {
// FIXME(eddyb) use lang items for methods instead of names.
let name = tcx.item_name(trait_item_id);
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_type_ir/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@ pub enum CandidateSource<I: Interner> {
#[derive(Clone, Copy, Hash, PartialEq, Eq, Debug)]
#[cfg_attr(feature = "nightly", derive(HashStable_NoContext, TyEncodable, TyDecodable))]
pub enum BuiltinImplSource {
/// A built-in impl that is considered trivial, without any nested requirements. They
/// are preferred over where-clauses, and we want to track them explicitly.
Trivial,
/// Some built-in impl we don't need to differentiate. This should be used
/// unless more specific information is necessary.
Misc,
Expand Down
2 changes: 2 additions & 0 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ use crate::utils::helpers::{self, exe, output, t};
#[rustfmt::skip] // We don't want rustfmt to oneline this list
pub(crate) const RUSTC_IF_UNCHANGED_ALLOWED_PATHS: &[&str] = &[
":!src/tools",
":!src/librustdoc",
":!src/rustdoc-json-types",
":!tests",
":!triagebot.toml",
];
Expand Down
4 changes: 2 additions & 2 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::{ColorConfig, json, stamp_file_path};
mod debugger;

// Helper modules that implement test running logic for each test suite.
// tidy-alphabet-start
// tidy-alphabetical-start
mod assembly;
mod codegen;
mod codegen_units;
Expand All @@ -47,7 +47,7 @@ mod run_make;
mod rustdoc;
mod rustdoc_json;
mod ui;
// tidy-alphabet-end
// tidy-alphabetical-end

#[cfg(test)]
mod tests;
Expand Down
94 changes: 66 additions & 28 deletions src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,21 @@ const ANNOTATIONS_TO_IGNORE: &[&str] = &[
"//@ normalize-stderr",
];

// If you edit this, also edit where it gets used in `check` (calling `contains_ignore_directives`)
const CONFIGURABLE_CHECKS: [&str; 11] = [
"cr",
"undocumented-unsafe",
"tab",
"linelength",
"filelength",
"end-whitespace",
"trailing-newlines",
"leading-newlines",
"copyright",
"dbg",
"odd-backticks",
];

fn generate_problems<'a>(
consts: &'a [u32],
letter_digit: &'a FxHashMap<char, char>,
Expand Down Expand Up @@ -220,6 +235,7 @@ fn long_line_is_ok(extension: &str, is_error_code: bool, max_columns: usize, lin
}
}

#[derive(Clone, Copy)]
enum Directive {
/// By default, tidy always warns against style issues.
Deny,
Expand All @@ -231,20 +247,28 @@ enum Directive {
Ignore(bool),
}

fn contains_ignore_directive(can_contain: bool, contents: &str, check: &str) -> Directive {
// Use a fixed size array in the return type to catch mistakes with changing `CONFIGURABLE_CHECKS`
// without changing the code in `check` easier.
fn contains_ignore_directives<const N: usize>(
can_contain: bool,
contents: &str,
checks: [&str; N],
) -> [Directive; N] {
if !can_contain {
return Directive::Deny;
}
// Update `can_contain` when changing this
if contents.contains(&format!("// ignore-tidy-{check}"))
|| contents.contains(&format!("# ignore-tidy-{check}"))
|| contents.contains(&format!("/* ignore-tidy-{check} */"))
|| contents.contains(&format!("<!-- ignore-tidy-{check} -->"))
{
Directive::Ignore(false)
} else {
Directive::Deny
return [Directive::Deny; N];
}
checks.map(|check| {
// Update `can_contain` when changing this
if contents.contains(&format!("// ignore-tidy-{check}"))
|| contents.contains(&format!("# ignore-tidy-{check}"))
|| contents.contains(&format!("/* ignore-tidy-{check} */"))
|| contents.contains(&format!("<!-- ignore-tidy-{check} -->"))
{
Directive::Ignore(false)
} else {
Directive::Deny
}
})
}

macro_rules! suppressible_tidy_err {
Expand Down Expand Up @@ -370,6 +394,7 @@ pub fn check(path: &Path, bad: &mut bool) {
COLS
};

// When you change this, also change the `directive_line_starts` variable below
let can_contain = contents.contains("// ignore-tidy-")
|| contents.contains("# ignore-tidy-")
|| contents.contains("/* ignore-tidy-")
Expand All @@ -385,22 +410,19 @@ pub fn check(path: &Path, bad: &mut bool) {
return;
}
}
let mut skip_cr = contains_ignore_directive(can_contain, &contents, "cr");
let mut skip_undocumented_unsafe =
contains_ignore_directive(can_contain, &contents, "undocumented-unsafe");
let mut skip_tab = contains_ignore_directive(can_contain, &contents, "tab");
let mut skip_line_length = contains_ignore_directive(can_contain, &contents, "linelength");
let mut skip_file_length = contains_ignore_directive(can_contain, &contents, "filelength");
let mut skip_end_whitespace =
contains_ignore_directive(can_contain, &contents, "end-whitespace");
let mut skip_trailing_newlines =
contains_ignore_directive(can_contain, &contents, "trailing-newlines");
let mut skip_leading_newlines =
contains_ignore_directive(can_contain, &contents, "leading-newlines");
let mut skip_copyright = contains_ignore_directive(can_contain, &contents, "copyright");
let mut skip_dbg = contains_ignore_directive(can_contain, &contents, "dbg");
let mut skip_odd_backticks =
contains_ignore_directive(can_contain, &contents, "odd-backticks");
let [
mut skip_cr,
mut skip_undocumented_unsafe,
mut skip_tab,
mut skip_line_length,
mut skip_file_length,
mut skip_end_whitespace,
mut skip_trailing_newlines,
mut skip_leading_newlines,
mut skip_copyright,
mut skip_dbg,
mut skip_odd_backticks,
] = contains_ignore_directives(can_contain, &contents, CONFIGURABLE_CHECKS);
let mut leading_new_lines = false;
let mut trailing_new_lines = 0;
let mut lines = 0;
Expand Down Expand Up @@ -474,6 +496,22 @@ pub fn check(path: &Path, bad: &mut bool) {
suppressible_tidy_err!(err, skip_cr, "CR character");
}
if !is_this_file {
let directive_line_starts = ["// ", "# ", "/* ", "<!-- "];
let possible_line_start =
directive_line_starts.into_iter().any(|s| line.starts_with(s));
let contains_potential_directive =
possible_line_start && (line.contains("-tidy") || line.contains("tidy-"));
let has_recognized_ignore_directive =
contains_ignore_directives(can_contain, line, CONFIGURABLE_CHECKS)
.into_iter()
.any(|directive| matches!(directive, Directive::Ignore(_)));
let has_alphabetical_directive = line.contains("tidy-alphabetical-start")
|| line.contains("tidy-alphabetical-end");
let has_recognized_directive =
has_recognized_ignore_directive || has_alphabetical_directive;
if contains_potential_directive && (!has_recognized_directive) {
err("Unrecognized tidy directive")
}
// Allow using TODO in diagnostic suggestions by marking the
// relevant line with `// ignore-tidy-todo`.
if trimmed.contains("TODO") && !trimmed.contains("ignore-tidy-todo") {
Expand Down
5 changes: 0 additions & 5 deletions tests/crashes/135341.rs

This file was deleted.

5 changes: 4 additions & 1 deletion tests/ui/regions/issue-26448-1.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
//@ run-pass
//@ revisions: current next
//@ [next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)
//@ check-pass

pub trait Foo<T> {
fn foo(self) -> T;
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/regions/issue-26448-2.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
//@ revisions: current next
//@ [next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)
//@ check-pass

pub struct Bar<T> {
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/wf/ice-hir-wf-issue-135341.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type A<T> = B;
type B = _; //~ ERROR the placeholder `_` is not allowed within types on item signatures for type aliases

fn main() {}
9 changes: 9 additions & 0 deletions tests/ui/wf/ice-hir-wf-issue-135341.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0121]: the placeholder `_` is not allowed within types on item signatures for type aliases
--> $DIR/ice-hir-wf-issue-135341.rs:2:10
|
LL | type B = _;
| ^ not allowed in type signatures

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0121`.

0 comments on commit 8321f00

Please sign in to comment.