Skip to content

Commit

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

Rollup of 5 pull requests

Successful merges:

 - rust-lang#134742 (Use `PostBorrowckAnalysis` in `check_coroutine_obligations`)
 - rust-lang#134951 (Suppress host effect predicates if underlying trait doesn't hold)
 - rust-lang#135097 (bootstrap: Consolidate coverage test suite steps into a single step)
 - rust-lang#135146 (Don't enable anyhow's `backtrace` feature in opt-dist)
 - rust-lang#135157 (Move the has_errors check in rustdoc back to after TyCtxt is created)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jan 6, 2025
2 parents 243d2ca + 09d0472 commit f010879
Show file tree
Hide file tree
Showing 11 changed files with 224 additions and 155 deletions.
3 changes: 0 additions & 3 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,6 @@ name = "anyhow"
version = "1.0.95"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "34ac096ce696dc2fcabef30516bb13c0a68a11d30131d3df6f04711467681b04"
dependencies = [
"backtrace",
]

[[package]]
name = "ar_archive_writer"
Expand Down
23 changes: 15 additions & 8 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1845,13 +1845,18 @@ pub(super) fn check_coroutine_obligations(

debug!(?typeck_results.coroutine_stalled_predicates);

let mode = if tcx.next_trait_solver_globally() {
TypingMode::post_borrowck_analysis(tcx, def_id)
} else {
TypingMode::analysis_in_body(tcx, def_id)
};

let infcx = tcx
.infer_ctxt()
// typeck writeback gives us predicates with their regions erased.
// As borrowck already has checked lifetimes, we do not need to do it again.
.ignoring_regions()
// FIXME(#132279): This should eventually use the already defined hidden types.
.build(TypingMode::analysis_in_body(tcx, def_id));
.build(mode);

let ocx = ObligationCtxt::new_with_diagnostics(&infcx);
for (predicate, cause) in &typeck_results.coroutine_stalled_predicates {
Expand All @@ -1864,12 +1869,14 @@ pub(super) fn check_coroutine_obligations(
return Err(infcx.err_ctxt().report_fulfillment_errors(errors));
}

// Check that any hidden types found when checking these stalled coroutine obligations
// are valid.
for (key, ty) in infcx.take_opaque_types() {
let hidden_type = infcx.resolve_vars_if_possible(ty.hidden_type);
let key = infcx.resolve_vars_if_possible(key);
sanity_check_found_hidden_type(tcx, key, hidden_type)?;
if !tcx.next_trait_solver_globally() {
// Check that any hidden types found when checking these stalled coroutine obligations
// are valid.
for (key, ty) in infcx.take_opaque_types() {
let hidden_type = infcx.resolve_vars_if_possible(ty.hidden_type);
let key = infcx.resolve_vars_if_possible(key);
sanity_check_found_hidden_type(tcx, key, hidden_type)?;
}
}

Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
//
// We rely on a few heuristics to identify cases where this root
// obligation is more important than the leaf obligation:
let (main_trait_predicate, o) = if let ty::PredicateKind::Clause(
let (main_trait_predicate, main_obligation) = if let ty::PredicateKind::Clause(
ty::ClauseKind::Trait(root_pred)
) = root_obligation.predicate.kind().skip_binder()
&& !leaf_trait_predicate.self_ty().skip_binder().has_escaping_bound_vars()
Expand Down Expand Up @@ -199,7 +199,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
notes,
parent_label,
append_const_msg,
} = self.on_unimplemented_note(main_trait_predicate, o, &mut long_ty_file);
} = self.on_unimplemented_note(main_trait_predicate, main_obligation, &mut long_ty_file);

let have_alt_message = message.is_some() || label.is_some();
let is_try_conversion = self.is_try_conversion(span, main_trait_ref.def_id());
Expand Down Expand Up @@ -538,23 +538,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}

ty::PredicateKind::Clause(ty::ClauseKind::HostEffect(predicate)) => {
// FIXME(const_trait_impl): We should recompute the predicate with `~const`
// if it's `const`, and if it holds, explain that this bound only
// *conditionally* holds. If that fails, we should also do selection
// to drill this down to an impl or built-in source, so we can
// point at it and explain that while the trait *is* implemented,
// that implementation is not const.
let err_msg = self.get_standard_error_message(
bound_predicate.rebind(ty::TraitPredicate {
trait_ref: predicate.trait_ref,
polarity: ty::PredicatePolarity::Positive,
}),
None,
Some(predicate.constness),
None,
String::new(),
);
struct_span_code_err!(self.dcx(), span, E0277, "{}", err_msg)
self.report_host_effect_error(bound_predicate.rebind(predicate), obligation.param_env, span)
}

ty::PredicateKind::Subtype(predicate) => {
Expand Down Expand Up @@ -763,6 +747,41 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
applied_do_not_recommend
}

fn report_host_effect_error(
&self,
predicate: ty::Binder<'tcx, ty::HostEffectPredicate<'tcx>>,
param_env: ty::ParamEnv<'tcx>,
span: Span,
) -> Diag<'a> {
// FIXME(const_trait_impl): We should recompute the predicate with `~const`
// if it's `const`, and if it holds, explain that this bound only
// *conditionally* holds. If that fails, we should also do selection
// to drill this down to an impl or built-in source, so we can
// point at it and explain that while the trait *is* implemented,
// that implementation is not const.
let trait_ref = predicate.map_bound(|predicate| ty::TraitPredicate {
trait_ref: predicate.trait_ref,
polarity: ty::PredicatePolarity::Positive,
});
let err_msg = self.get_standard_error_message(
trait_ref,
None,
Some(predicate.constness()),
None,
String::new(),
);
let mut diag = struct_span_code_err!(self.dcx(), span, E0277, "{}", err_msg);
if !self.predicate_may_hold(&Obligation::new(
self.tcx,
ObligationCause::dummy(),
param_env,
trait_ref,
)) {
diag.downgrade_to_delayed_bug();
}
diag
}

fn emit_specialized_closure_kind_error(
&self,
obligation: &PredicateObligation<'tcx>,
Expand Down
186 changes: 68 additions & 118 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::utils::helpers::{
linker_args, linker_flags, t, target_supports_cranelift_backend, up_to_date,
};
use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests};
use crate::{CLang, DocTests, GitRepo, Mode, envify};
use crate::{CLang, DocTests, GitRepo, Mode, PathSet, envify};

const ADB_TEST_DIR: &str = "/data/local/tmp/work";

Expand Down Expand Up @@ -1185,53 +1185,6 @@ macro_rules! test {
};
}

/// Declares an alias for running the [`Coverage`] tests in only one mode.
/// Adapted from [`test`].
macro_rules! coverage_test_alias {
(
$( #[$attr:meta] )* // allow docstrings and attributes
$name:ident {
alias_and_mode: $alias_and_mode:expr, // &'static str
default: $default:expr, // bool
only_hosts: $only_hosts:expr // bool
$( , )? // optional trailing comma
}
) => {
$( #[$attr] )*
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct $name {
pub compiler: Compiler,
pub target: TargetSelection,
}

impl $name {
const MODE: &'static str = $alias_and_mode;
}

impl Step for $name {
type Output = ();
const DEFAULT: bool = $default;
const ONLY_HOSTS: bool = $only_hosts;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
// Register the mode name as a command-line alias.
// This allows `x test coverage-map` and `x test coverage-run`.
run.alias($alias_and_mode)
}

fn make_run(run: RunConfig<'_>) {
let compiler = run.builder.compiler(run.builder.top_stage, run.build_triple());

run.builder.ensure($name { compiler, target: run.target });
}

fn run(self, builder: &Builder<'_>) {
Coverage::run_coverage_tests(builder, self.compiler, self.target, Self::MODE);
}
}
};
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
pub struct RunMakeSupport {
pub compiler: Compiler,
Expand Down Expand Up @@ -1473,99 +1426,96 @@ impl Step for RunMake {

test!(Assembly { path: "tests/assembly", mode: "assembly", suite: "assembly", default: true });

/// Coverage tests are a bit more complicated than other test suites, because
/// we want to run the same set of test files in multiple different modes,
/// in a way that's convenient and flexible when invoked manually.
///
/// This combined step runs the specified tests (or all of `tests/coverage`)
/// in both "coverage-map" and "coverage-run" modes.
///
/// Used by:
/// - `x test coverage`
/// - `x test tests/coverage`
/// - `x test tests/coverage/trivial.rs` (etc)
///
/// (Each individual mode also has its own step that will run the tests in
/// just that mode.)
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
/// Runs the coverage test suite at `tests/coverage` in some or all of the
/// coverage test modes.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Coverage {
pub compiler: Compiler,
pub target: TargetSelection,
pub mode: &'static str,
}

impl Coverage {
const PATH: &'static str = "tests/coverage";
const SUITE: &'static str = "coverage";

/// Runs the coverage test suite (or a user-specified subset) in one mode.
///
/// This same function is used by the multi-mode step ([`Coverage`]) and by
/// the single-mode steps ([`CoverageMap`] and [`CoverageRun`]), to help
/// ensure that they all behave consistently with each other, regardless of
/// how the coverage tests have been invoked.
fn run_coverage_tests(
builder: &Builder<'_>,
compiler: Compiler,
target: TargetSelection,
mode: &'static str,
) {
// Like many other test steps, we delegate to a `Compiletest` step to
// actually run the tests. (See `test_definitions!`.)
builder.ensure(Compiletest {
compiler,
target,
mode,
suite: Self::SUITE,
path: Self::PATH,
compare_mode: None,
});
}
const ALL_MODES: &[&str] = &["coverage-map", "coverage-run"];
}

impl Step for Coverage {
type Output = ();
/// We rely on the individual CoverageMap/CoverageRun steps to run themselves.
const DEFAULT: bool = false;
/// When manually invoked, try to run as much as possible.
const DEFAULT: bool = true;
/// Compiletest will automatically skip the "coverage-run" tests if necessary.
const ONLY_HOSTS: bool = false;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
// Take responsibility for command-line paths within `tests/coverage`.
run.suite_path(Self::PATH)
fn should_run(mut run: ShouldRun<'_>) -> ShouldRun<'_> {
// Support various invocation styles, including:
// - `./x test coverage`
// - `./x test tests/coverage/trivial.rs`
// - `./x test coverage-map`
// - `./x test coverage-run -- tests/coverage/trivial.rs`
run = run.suite_path(Self::PATH);
for mode in Self::ALL_MODES {
run = run.alias(mode);
}
run
}

fn make_run(run: RunConfig<'_>) {
let compiler = run.builder.compiler(run.builder.top_stage, run.build_triple());
let target = run.target;

// List of (coverage) test modes that the coverage test suite will be
// run in. It's OK for this to contain duplicates, because the call to
// `Builder::ensure` below will take care of deduplication.
let mut modes = vec![];

// From the pathsets that were selected on the command-line (or by default),
// determine which modes to run in.
for path in &run.paths {
match path {
PathSet::Set(_) => {
for mode in Self::ALL_MODES {
if path.assert_single_path().path == Path::new(mode) {
modes.push(mode);
break;
}
}
}
PathSet::Suite(_) => {
modes.extend(Self::ALL_MODES);
break;
}
}
}

// Skip any modes that were explicitly skipped/excluded on the command-line.
// FIXME(Zalathar): Integrate this into central skip handling somehow?
modes.retain(|mode| !run.builder.config.skip.iter().any(|skip| skip == Path::new(mode)));

// FIXME(Zalathar): Make these commands skip all coverage tests, as expected:
// - `./x test --skip=tests`
// - `./x test --skip=tests/coverage`
// - `./x test --skip=coverage`
// Skip handling currently doesn't have a way to know that skipping the coverage
// suite should also skip the `coverage-map` and `coverage-run` aliases.

run.builder.ensure(Coverage { compiler, target: run.target });
for mode in modes {
run.builder.ensure(Coverage { compiler, target, mode });
}
}

fn run(self, builder: &Builder<'_>) {
// Run the specified coverage tests (possibly all of them) in both modes.
Self::run_coverage_tests(builder, self.compiler, self.target, CoverageMap::MODE);
Self::run_coverage_tests(builder, self.compiler, self.target, CoverageRun::MODE);
}
}

coverage_test_alias! {
/// Runs the `tests/coverage` test suite in "coverage-map" mode only.
/// Used by `x test` and `x test coverage-map`.
CoverageMap {
alias_and_mode: "coverage-map",
default: true,
only_hosts: false,
}
}
coverage_test_alias! {
/// Runs the `tests/coverage` test suite in "coverage-run" mode only.
/// Used by `x test` and `x test coverage-run`.
CoverageRun {
alias_and_mode: "coverage-run",
default: true,
// Compiletest knows how to automatically skip these tests when cross-compiling,
// but skipping the whole step here makes it clearer that they haven't run at all.
only_hosts: true,
let Self { compiler, target, mode } = self;
// Like other compiletest suite test steps, delegate to an internal
// compiletest task to actually run the tests.
builder.ensure(Compiletest {
compiler,
target,
mode,
suite: Self::SUITE,
path: Self::PATH,
compare_mode: None,
});
}
}

Expand Down
2 changes: 0 additions & 2 deletions src/bootstrap/src/core/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,8 +944,6 @@ impl<'a> Builder<'a> {
test::Ui,
test::Crashes,
test::Coverage,
test::CoverageMap,
test::CoverageRun,
test::MirOpt,
test::Codegen,
test::CodegenUnits,
Expand Down
Loading

0 comments on commit f010879

Please sign in to comment.