Skip to content

Experiment: Apply #[deprecated_safe] to env::set_var/env::remove_var #95942

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

Closed
wants to merge 8 commits into from
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4457,6 +4457,7 @@ dependencies = [
"rustc_index",
"rustc_infer",
"rustc_lint",
"rustc_lint_defs",
"rustc_macros",
"rustc_middle",
"rustc_serialize",
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2011,7 +2011,10 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
}

CastKind::Pointer(PointerCast::UnsafeFnPointer) => {
CastKind::Pointer(
ptr_cast @ (PointerCast::UnsafeFnPointer
| PointerCast::DeprecatedSafeFnPointer),
) => {
let fn_sig = op.ty(body, tcx).fn_sig(tcx);

// The type that we see in the fcx is like
Expand All @@ -2021,7 +2024,13 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
// and hence may contain unnormalized results.
let fn_sig = self.normalize(fn_sig, location);

let ty_fn_ptr_from = tcx.safe_to_unsafe_fn_ty(fn_sig);
let ty_fn_ptr_from = match ptr_cast {
PointerCast::UnsafeFnPointer => tcx.safe_to_unsafe_fn_ty(fn_sig),
PointerCast::DeprecatedSafeFnPointer => {
tcx.unsafe_to_safe_fn_ty(fn_sig)
}
_ => unreachable!(),
};

if let Err(terr) = self.eq_types(
*ty,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,11 @@ fn codegen_stmt<'tcx>(
ref operand,
to_ty,
)
| Rvalue::Cast(
CastKind::Pointer(PointerCast::DeprecatedSafeFnPointer),
ref operand,
to_ty,
)
| Rvalue::Cast(
CastKind::Pointer(PointerCast::MutToConstPointer),
ref operand,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ impl rustc_driver::Callbacks for CraneliftPassesCallbacks {

fn main() {
rustc_driver::init_rustc_env_logger();
rustc_driver::install_ice_hook();
// SAFETY: in main(), no other threads could be reading or writing the environment
// FIXME(skippy) are there definitely zero threads here? other functions have been called
unsafe {
rustc_driver::install_ice_hook();
}
let exit_code = rustc_driver::catch_with_exit_code(|| {
let mut use_clif = false;

Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// This is a no-op at the LLVM level.
operand.val
}
mir::CastKind::Pointer(PointerCast::DeprecatedSafeFnPointer) => {
// This is a no-op at the LLVM level.
operand.val
}
mir::CastKind::Pointer(PointerCast::Unsize) => {
assert!(bx.cx().is_backend_scalar_pair(cast));
let (lldata, llextra) = match operand.val {
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_const_eval/src/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

Pointer(PointerCast::DeprecatedSafeFnPointer) => {
let src = self.read_immediate(src)?;
match cast_ty.kind() {
ty::FnPtr(_) => {
// No change to value
self.write_immediate(*src, dest)?;
}
_ => span_bug!(self.cur_span(), "unsafe fn to fn cast on {:?}", cast_ty),
}
}

Pointer(PointerCast::ClosureFnPointer(_)) => {
// The src operand does not matter, just its type
match *src.layout.ty.kind() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
Rvalue::Cast(
CastKind::Pointer(
PointerCast::UnsafeFnPointer
| PointerCast::DeprecatedSafeFnPointer
| PointerCast::ClosureFnPointer(_)
| PointerCast::ReifyFnPointer,
),
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,11 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
/// Installs a panic hook that will print the ICE message on unexpected panics.
///
/// A custom rustc driver can skip calling this to set up a custom ICE hook.
pub fn install_ice_hook() {
///
/// # Safety
///
/// Must not be called when any other thread could be reading or writing the environment.
pub unsafe fn install_ice_hook() {
// If the user has not explicitly overridden "RUST_BACKTRACE", then produce
// full backtraces. When a compiler ICE happens, we want to gather
// as much information as possible to present in the issue opened
Expand Down Expand Up @@ -1320,7 +1324,11 @@ pub fn main() -> ! {
init_rustc_env_logger();
signal_handler::install();
let mut callbacks = TimePassesCallbacks::default();
install_ice_hook();
// SAFETY: in main(), no other threads could be reading or writing the environment
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// SAFETY: Lack of thread conflicts asserted by the caller

// FIXME(skippy) are there definitely zero threads here? other functions have been called
unsafe {
install_ice_hook();
}
let exit_code = catch_with_exit_code(|| {
let args = env::args_os()
.enumerate()
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,13 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
),
// lang-team MCP 147
gated!(
deprecated_safe, Normal, template!(List: r#"since = "version", note = "...""#), ErrorFollowing,
deprecated_safe, Normal,
template!(
Word,
List: r#"/*opt*/ since = "version", /*opt*/ note = "reason", /*opt*/ unsafe_edition = "edition""#,
NameValueStr: "reason"
),
ErrorFollowing,
experimental!(deprecated_safe),
),

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use super::UnstableFeatures;
#[test]
fn rustc_bootstrap_parsing() {
let is_bootstrap = |env, krate| {
// FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
#[cfg_attr(not(bootstrap), allow(deprecated_safe))]
std::env::set_var("RUSTC_BOOTSTRAP", env);
matches!(UnstableFeatures::from_environment(krate), UnstableFeatures::Cheat)
};
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err_count_on_creation: self.err_count_on_creation,
in_snapshot: self.in_snapshot.clone(),
universe: self.universe.clone(),
query_mode: self.query_mode.clone(),
}
}
}
Expand Down
47 changes: 42 additions & 5 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,30 @@ pub struct InferCtxt<'a, 'tcx> {
/// when we enter into a higher-ranked (`for<..>`) type or trait
/// bound.
universe: Cell<ty::UniverseIndex>,

// FIXME(skippy) specifically needs reviewer feedback
// FIXME(skippy) this is added so that i can detect when enter_with_canonical()
// is used, as a way to know that obligations will be coming in
// with bad cause spans that can't be used
// FIXME(skippy) what i really want (i think) is something like
// if !during_borrowck && !during_codegen
/// The mode that trait queries run in, which informs our error handling
/// policy. In essence, canonicalized queries need their errors propagated
/// rather than immediately reported because we do not have accurate spans.
pub query_mode: TraitQueryMode,
}

/// The mode that trait queries run in.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum TraitQueryMode {
/// Standard/un-canonicalized queries get accurate
/// spans etc. passed in and hence can do reasonable
/// error reporting on their own.
Standard,
/// Canonicalized queries get dummy spans and hence
/// must generally propagate errors to
/// pre-canonicalization callsites.
Canonical,
}

/// See the `error_reporting` module for more details.
Expand Down Expand Up @@ -615,14 +639,26 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
where
T: TypeFoldable<'tcx>,
{
self.enter(|infcx| {
let (value, subst) =
infcx.instantiate_canonical_with_fresh_inference_vars(span, canonical);
f(infcx, value, subst)
})
self.enter_with_query_mode(
|infcx| {
let (value, subst) =
infcx.instantiate_canonical_with_fresh_inference_vars(span, canonical);
f(infcx, value, subst)
},
TraitQueryMode::Canonical,
)
}

pub fn enter<R>(&mut self, f: impl for<'a> FnOnce(InferCtxt<'a, 'tcx>) -> R) -> R {
self.enter_with_query_mode(f, TraitQueryMode::Standard)
}

// FIXME(skippy) specifically needs reviewer feedback (see query_mode field)
fn enter_with_query_mode<R>(
&mut self,
f: impl for<'a> FnOnce(InferCtxt<'a, 'tcx>) -> R,
query_mode: TraitQueryMode,
) -> R {
let InferCtxtBuilder { tcx, defining_use_anchor, ref fresh_typeck_results } = *self;
let in_progress_typeck_results = fresh_typeck_results.as_ref();
f(InferCtxt {
Expand All @@ -640,6 +676,7 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
in_snapshot: Cell::new(false),
skip_leak_check: Cell::new(false),
universe: Cell::new(ty::UniverseIndex::ROOT),
query_mode,
})
}
}
Expand Down
24 changes: 16 additions & 8 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,13 +315,17 @@ pub fn configure_and_expand(
new_path.push(path);
}
}
env::set_var(
"PATH",
&env::join_paths(
new_path.iter().filter(|p| env::join_paths(iter::once(p)).is_ok()),
)
.unwrap(),
);
// SAFETY: we're in a cfg!(windows) section, set_var is always sound
#[cfg_attr(bootstrap, allow(unused_unsafe))]
unsafe {
env::set_var(
"PATH",
&env::join_paths(
new_path.iter().filter(|p| env::join_paths(iter::once(p)).is_ok()),
)
.unwrap(),
);
}
}

// Create the config for macro expansion
Expand Down Expand Up @@ -367,7 +371,11 @@ pub fn configure_and_expand(
resolver.lint_buffer().buffer_lint(lint, node_id, span, msg);
}
if cfg!(windows) {
env::set_var("PATH", &old_path);
// SAFETY: we're in a cfg!(windows) section, set_var is always sound
#[cfg_attr(bootstrap, allow(unused_unsafe))]
unsafe {
env::set_var("PATH", &old_path);
}
}

if recursion_limit_hit {
Expand Down
61 changes: 61 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3129,6 +3129,8 @@ declare_lint_pass! {
UNEXPECTED_CFGS,
DEPRECATED_WHERE_CLAUSE_LOCATION,
TEST_UNSTABLE_LINT,
DEPRECATED_SAFE,
DEPRECATED_SAFE_IN_FUTURE,
]
}

Expand Down Expand Up @@ -3794,3 +3796,62 @@ declare_lint! {
"this unstable lint is only for testing",
@feature_gate = sym::test_unstable_lint;
}

declare_lint! {
/// The `deprecated_safe` lint detects safe, unsound usage of items that are now marked ```[unsafe]```,
/// with safe usage being deprecated.
///
/// ### Example
///
/// ```rust
/// #![feature(deprecated_safe)]
///
/// #[deprecated_safe(since = "1.61.0", note = "reason")]
/// unsafe fn previously_safe_function() {}
///
/// fn main() {
/// previously_safe_function();
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// FIXME(skippy) broken link: [`deprecated_safe` attribute]
/// Items may be marked "deprecated_safe" with the [`deprecated_safe` attribute] to
/// indicate that they should no longer be used. Usually the attribute
/// should include a note on what to use instead, or check the
/// documentation.
///
/// [`deprecated_safe` attribute]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated_safe-attribute
/// [`unsafe fn`]: https://doc.rust-lang.org/reference/unsafe-functions.html
/// [`unsafe` trait]: https://doc.rust-lang.org/reference/items/traits.html#unsafe-traits
/// [`unsafe` block]: https://doc.rust-lang.org/reference/expressions/block-expr.html#unsafe-blocks
/// [unsafe]: https://doc.rust-lang.org/reference/unsafety.html
pub DEPRECATED_SAFE,
Warn,
"detects unsound use of items that are now marked unsafe, when safe use is deprecated",
report_in_external_macro
// FIXME(skippy) use future_incompatible?
/*
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #94978 <https://github.com/rust-lang/rust/issues/94978>",
};
*/
}

declare_lint! {
/// The `deprecated_safe_in_future` lint is internal to rustc and should not be
/// used by user code.
///
/// This lint is only enabled in the standard library. It works with the
/// use of `#[deprecated_safe]` with a `since` field of a version in the
/// future. This allows something to be marked as deprecated_safe in a future
/// version, and then this lint will ensure that the item is no longer
/// used as safe in the standard library.
pub DEPRECATED_SAFE_IN_FUTURE,
Allow,
"detects use of items that will be deprecated_safe in a future version",
report_in_external_macro
}
8 changes: 6 additions & 2 deletions compiler/rustc_llvm/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ fn detect_llvm_link() -> (&'static str, &'static str) {
// the one we want to use. As such, we restore the environment to what bootstrap saw. This isn't
// perfect -- we might actually want to see something from Cargo's added library paths -- but
// for now it works.
fn restore_library_path() {
// SAFETY: must not be called when any other thread could be reading or writing the environment
unsafe fn restore_library_path() {
let key = tracked_env_var_os("REAL_LIBRARY_PATH_VAR").expect("REAL_LIBRARY_PATH_VAR");
if let Some(env) = tracked_env_var_os("REAL_LIBRARY_PATH") {
env::set_var(&key, &env);
Expand Down Expand Up @@ -81,7 +82,10 @@ fn main() {
return;
}

restore_library_path();
// SAFETY: in main(), no other threads could be reading or writing the environment
unsafe {
restore_library_path();
}

let target = env::var("TARGET").expect("TARGET was not set");
let llvm_config =
Expand Down
Loading