Skip to content

Disable visible path calculation for PrettyPrinter in Ok path of compiler #89120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions compiler/rustc_codegen_llvm/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::type_::Type;
use rustc_codegen_ssa::traits::*;
use rustc_middle::bug;
use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, TyAndLayout};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
use rustc_middle::ty::{self, Ty, TypeFoldable};
use rustc_target::abi::{Abi, AddressSpace, Align, FieldsShape};
use rustc_target::abi::{Int, Pointer, F32, F64};
Expand Down Expand Up @@ -43,7 +43,8 @@ fn uncached_llvm_type<'a, 'tcx>(
// in problematically distinct types due to HRTB and subtyping (see #47638).
// ty::Dynamic(..) |
ty::Adt(..) | ty::Closure(..) | ty::Foreign(..) | ty::Generator(..) | ty::Str => {
let mut name = with_no_trimmed_paths(|| layout.ty.to_string());
let mut name =
with_no_visible_paths(|| with_no_trimmed_paths(|| layout.ty.to_string()));
if let (&ty::Adt(def, _), &Variants::Single { index }) =
(layout.ty.kind(), &layout.variants)
{
Expand Down
25 changes: 15 additions & 10 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_index::vec::Idx;
use rustc_middle::mir::AssertKind;
use rustc_middle::mir::{self, SwitchTargets};
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
use rustc_span::source_map::Span;
use rustc_span::{sym, Symbol};
Expand Down Expand Up @@ -476,15 +476,20 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
UninitValid => !layout.might_permit_raw_init(bx, /*zero:*/ false),
};
if do_panic {
let msg_str = with_no_trimmed_paths(|| {
if layout.abi.is_uninhabited() {
// Use this error even for the other intrinsics as it is more precise.
format!("attempted to instantiate uninhabited type `{}`", ty)
} else if intrinsic == ZeroValid {
format!("attempted to zero-initialize type `{}`, which is invalid", ty)
} else {
format!("attempted to leave type `{}` uninitialized, which is invalid", ty)
}
let msg_str = with_no_visible_paths(|| {
with_no_trimmed_paths(|| {
if layout.abi.is_uninhabited() {
// Use this error even for the other intrinsics as it is more precise.
format!("attempted to instantiate uninhabited type `{}`", ty)
} else if intrinsic == ZeroValid {
format!("attempted to zero-initialize type `{}`, which is invalid", ty)
} else {
format!(
"attempted to leave type `{}` uninitialized, which is invalid",
ty
)
}
})
});
let msg = bx.const_str(Symbol::intern(&msg_str));
let location = self.get_caller_location(bx, source_info).immediate();
Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ thread_local! {
static SHOULD_PREFIX_WITH_CRATE: Cell<bool> = const { Cell::new(false) };
static NO_TRIMMED_PATH: Cell<bool> = const { Cell::new(false) };
static NO_QUERIES: Cell<bool> = const { Cell::new(false) };
static NO_VISIBLE_PATH: Cell<bool> = const { Cell::new(false) };
}

/// Avoids running any queries during any prints that occur
Expand Down Expand Up @@ -112,6 +113,16 @@ pub fn with_no_trimmed_paths<F: FnOnce() -> R, R>(f: F) -> R {
})
}

/// Prevent selection of visible paths. `Display` impl of DefId will prefer visible (public) reexports of types as paths.
pub fn with_no_visible_paths<F: FnOnce() -> R, R>(f: F) -> R {
NO_VISIBLE_PATH.with(|flag| {
let old = flag.replace(true);
let result = f();
flag.set(old);
result
})
}

/// The "region highlights" are used to control region printing during
/// specific error messages. When a "region highlight" is enabled, it
/// gives an alternate way to print specific regions. For now, we
Expand Down Expand Up @@ -268,6 +279,10 @@ pub trait PrettyPrinter<'tcx>:
/// from at least one local module, and returns `true`. If the crate defining `def_id` is
/// declared with an `extern crate`, the path is guaranteed to use the `extern crate`.
fn try_print_visible_def_path(self, def_id: DefId) -> Result<(Self, bool), Self::Error> {
if NO_VISIBLE_PATH.with(|flag| flag.get()) {
return Ok((self, false));
}

let mut callers = Vec::new();
self.try_print_visible_def_path_recur(def_id, &mut callers)
}
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_query_impl/src/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,13 @@ macro_rules! define_queries {
pub fn $name<$tcx>(tcx: QueryCtxt<$tcx>, key: query_keys::$name<$tcx>) -> QueryStackFrame {
let kind = dep_graph::DepKind::$name;
let name = stringify!($name);
let description = ty::print::with_forced_impl_filename_line(
// Disable visible paths printing for performance reasons.
// Showing visible path instead of any path is not that important in production.
let description = ty::print::with_no_visible_paths(
|| ty::print::with_forced_impl_filename_line(
// Force filename-line mode to avoid invoking `type_of` query.
|| queries::$name::describe(tcx, key)
);
));
let description = if tcx.sess.verbose() {
format!("{} [{}]", description, name)
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/impl-trait/auto-trait-leak.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ note: ...which requires type-checking `cycle1`...
|
LL | send(cycle2().clone());
| ^^^^
= note: ...which requires evaluating trait selection obligation `impl std::clone::Clone: std::marker::Send`...
= note: ...which requires evaluating trait selection obligation `impl core::clone::Clone: core::marker::Send`...
note: ...which requires computing type of `cycle2::{opaque#0}`...
--> $DIR/auto-trait-leak.rs:19:16
|
Expand Down Expand Up @@ -70,7 +70,7 @@ note: ...which requires type-checking `cycle2`...
|
LL | send(cycle1().clone());
| ^^^^
= note: ...which requires evaluating trait selection obligation `impl std::clone::Clone: std::marker::Send`...
= note: ...which requires evaluating trait selection obligation `impl core::clone::Clone: core::marker::Send`...
= note: ...which again requires computing type of `cycle1::{opaque#0}`, completing the cycle
note: cycle used when checking item types in top-level module
--> $DIR/auto-trait-leak.rs:1:1
Expand Down
11 changes: 6 additions & 5 deletions src/test/ui/intrinsics/panic-uninitialized-zeroed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// ignore-wasm32-bare compiled with panic=abort by default
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck
// ignore-tidy-linelength

// This test checks panic emitted from `mem::{uninitialized,zeroed}`.

Expand Down Expand Up @@ -114,11 +115,11 @@ fn main() {

test_panic_msg(
|| mem::uninitialized::<*const dyn Send>(),
"attempted to leave type `*const dyn std::marker::Send` uninitialized, which is invalid"
"attempted to leave type `*const dyn core::marker::Send` uninitialized, which is invalid"
);
test_panic_msg(
|| mem::zeroed::<*const dyn Send>(),
"attempted to zero-initialize type `*const dyn std::marker::Send`, which is invalid"
"attempted to zero-initialize type `*const dyn core::marker::Send`, which is invalid"
);

/* FIXME(#66151) we conservatively do not error here yet.
Expand All @@ -145,12 +146,12 @@ fn main() {

test_panic_msg(
|| mem::uninitialized::<(NonNull<u32>, u32, u32)>(),
"attempted to leave type `(std::ptr::NonNull<u32>, u32, u32)` uninitialized, \
"attempted to leave type `(core::ptr::non_null::NonNull<u32>, u32, u32)` uninitialized, \
which is invalid"
);
test_panic_msg(
|| mem::zeroed::<(NonNull<u32>, u32, u32)>(),
"attempted to zero-initialize type `(std::ptr::NonNull<u32>, u32, u32)`, \
"attempted to zero-initialize type `(core::ptr::non_null::NonNull<u32>, u32, u32)`, \
which is invalid"
);

Expand Down Expand Up @@ -187,7 +188,7 @@ fn main() {
);
test_panic_msg(
|| mem::uninitialized::<ManuallyDrop<LR>>(),
"attempted to leave type `std::mem::ManuallyDrop<LR>` uninitialized, which is invalid"
"attempted to leave type `core::mem::manually_drop::ManuallyDrop<LR>` uninitialized, which is invalid"
);

// Some things that should work.
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/recursion/issue-26548-recursion-via-normalize.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//~ ERROR cycle detected when computing layout of `S`
//~| NOTE ...which requires computing layout of `std::option::Option<<S as Mirror>::It>`...
//~| NOTE ...which requires computing layout of `std::option::Option<S>`...
//~| NOTE ...which requires computing layout of `core::option::Option<<S as Mirror>::It>`...
//~| NOTE ...which requires computing layout of `core::option::Option<S>`...
//~| NOTE ...which again requires computing layout of `S`, completing the cycle
//~| NOTE cycle used when computing layout of `std::option::Option<S>`
//~| NOTE cycle used when computing layout of `core::option::Option<S>`

// build-fail

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
error[E0391]: cycle detected when computing layout of `S`
|
= note: ...which requires computing layout of `std::option::Option<<S as Mirror>::It>`...
= note: ...which requires computing layout of `std::option::Option<S>`...
= note: ...which requires computing layout of `core::option::Option<<S as Mirror>::It>`...
= note: ...which requires computing layout of `core::option::Option<S>`...
= note: ...which again requires computing layout of `S`, completing the cycle
= note: cycle used when computing layout of `std::option::Option<S>`
= note: cycle used when computing layout of `core::option::Option<S>`

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ note: ...which requires type-checking `m::bar`...
|
LL | is_send(foo());
| ^^^^^^^
= note: ...which requires evaluating trait selection obligation `impl std::fmt::Debug: std::marker::Send`...
= note: ...which requires evaluating trait selection obligation `impl core::fmt::Debug: core::marker::Send`...
= note: ...which again requires computing type of `m::Foo::{opaque#0}`, completing the cycle
note: cycle used when checking item types in module `m`
--> $DIR/auto-trait-leakage3.rs:6:1
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/type-alias-impl-trait/inference-cycle.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ note: ...which requires type-checking `m::bar`...
|
LL | is_send(foo()); // Today: error
| ^^^^^^^
= note: ...which requires evaluating trait selection obligation `impl std::fmt::Debug: std::marker::Send`...
= note: ...which requires evaluating trait selection obligation `impl core::fmt::Debug: core::marker::Send`...
= note: ...which again requires computing type of `m::Foo::{opaque#0}`, completing the cycle
note: cycle used when checking item types in module `m`
--> $DIR/inference-cycle.rs:4:1
Expand Down