Skip to content

Commit

Permalink
Auto merge of rust-lang#100591 - est31:stabilization_placeholder, r=M…
Browse files Browse the repository at this point in the history
…ark-Simulacrum

Require stabilizations to use a placeholder instead of writing out stabilization version

Implements the idea from [this](https://rust-lang.zulipchat.com/#narrow/stream/241545-t-release/topic/libs.20stabilization.20placeholder) zulip stream.

It's a common phenomenon that feature stabilizations don't make it into a particular release, but the version is still inaccurate. Often this is caught in the PR, but it can also require subsequent changes to adjust/correct the version. A list with examples of such PRs is given in rust-lang#100577, but it's far from complete.

This PR requires stabilization PRs to use the placeholder `CURRENT_RUSTC_VERSION`, enforced via tidy tooling. The PR also adds a tool that replaces the placeholder with the version number. It can be invoked via `./x.py run src/tools/replace-version-placeholder` and is supposed to be ran upon beta branching as well as version bumping and any backports to the beta branch.  I filed PRs to the dev guide and forge to document these changes in the release and stabilization workflows:

* The [dev guide](https://rustc-dev-guide.rust-lang.org/stabilization_guide.html#determining-the-stabilization-version) PR: rust-lang/rustc-dev-guide#1443
* The [std dev guide](https://std-dev-guide.rust-lang.org/) PR: rust-lang/std-dev-guide#43
* The [forge](https://github.com/rust-lang/rust-forge) PR: rust-lang/rust-forge#643

Alternative to rust-lang#100577 which added checking.
  • Loading branch information
bors committed Aug 27, 2022
2 parents 332cc8f + d32ff14 commit eaadb89
Show file tree
Hide file tree
Showing 17 changed files with 271 additions and 86 deletions.
8 changes: 8 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3293,6 +3293,14 @@ dependencies = [
"winapi",
]

[[package]]
name = "replace-version-placeholder"
version = "0.1.0"
dependencies = [
"tidy",
"walkdir",
]

[[package]]
name = "rls"
version = "1.41.0"
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ members = [
"src/tools/jsondocck",
"src/tools/html-checker",
"src/tools/bump-stage0",
"src/tools/replace-version-placeholder",
"src/tools/lld-wrapper",
]

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_feature/src/accepted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ declare_features! (
/// especially around globs and shadowing (RFC 1560).
(accepted, item_like_imports, "1.15.0", Some(35120), None),
/// Allows `'a: { break 'a; }`.
(accepted, label_break_value, "1.65.0", Some(48594), None),
(accepted, label_break_value, "CURRENT_RUSTC_VERSION", Some(48594), None),
/// Allows `if/while p && let q = r && ...` chains.
(accepted, let_chains, "1.64.0", Some(53667), None),
/// Allows `break {expr}` with a value inside `loop`s.
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_passes/src/lib_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ impl<'tcx> LibFeatureCollector<'tcx> {
}
}
}
const VERSION_PLACEHOLDER: &str = "CURRENT_RUSTC_VERSION";

if let Some(s) = since && s.as_str() == VERSION_PLACEHOLDER {
let version = option_env!("CFG_VERSION").unwrap_or("<current>");
let version = version.split(' ').next().unwrap();
since = Some(Symbol::intern(&version));
}

if let Some(feature) = feature {
// This additional check for stability is to make sure we
// don't emit additional, irrelevant errors for malformed
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ impl<T: ?Sized> *const T {
///
/// This is a bit safer than `as` because it wouldn't silently change the type if the code is
/// refactored.
#[stable(feature = "ptr_const_cast", since = "1.65.0")]
#[rustc_const_stable(feature = "ptr_const_cast", since = "1.65.0")]
#[stable(feature = "ptr_const_cast", since = "CURRENT_RUSTC_VERSION")]
#[rustc_const_stable(feature = "ptr_const_cast", since = "CURRENT_RUSTC_VERSION")]
pub const fn cast_mut(self) -> *mut T {
self as _
}
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/ptr/mut_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ impl<T: ?Sized> *mut T {
/// coercion.
///
/// [`cast_mut`]: #method.cast_mut
#[stable(feature = "ptr_const_cast", since = "1.65.0")]
#[rustc_const_stable(feature = "ptr_const_cast", since = "1.65.0")]
#[stable(feature = "ptr_const_cast", since = "CURRENT_RUSTC_VERSION")]
#[rustc_const_stable(feature = "ptr_const_cast", since = "CURRENT_RUSTC_VERSION")]
pub const fn cast_const(self) -> *const T {
self as _
}
Expand Down
26 changes: 13 additions & 13 deletions library/std/src/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
//! `RUST_LIB_BACKTRACE` or `RUST_BACKTRACE` at runtime might not actually change
//! how backtraces are captured.
#![stable(feature = "backtrace", since = "1.65.0")]
#![stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -104,29 +104,29 @@ use crate::vec::Vec;
/// previous point in time. In some instances the `Backtrace` type may
/// internally be empty due to configuration. For more information see
/// `Backtrace::capture`.
#[stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
#[must_use]
pub struct Backtrace {
inner: Inner,
}

/// The current status of a backtrace, indicating whether it was captured or
/// whether it is empty for some other reason.
#[stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
#[non_exhaustive]
#[derive(Debug, PartialEq, Eq)]
pub enum BacktraceStatus {
/// Capturing a backtrace is not supported, likely because it's not
/// implemented for the current platform.
#[stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
Unsupported,
/// Capturing a backtrace has been disabled through either the
/// `RUST_LIB_BACKTRACE` or `RUST_BACKTRACE` environment variables.
#[stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
Disabled,
/// A backtrace has been captured and the `Backtrace` should print
/// reasonable information when rendered.
#[stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
Captured,
}

Expand Down Expand Up @@ -173,7 +173,7 @@ enum BytesOrWide {
Wide(Vec<u16>),
}

#[stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
impl fmt::Debug for Backtrace {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
let capture = match &self.inner {
Expand Down Expand Up @@ -289,7 +289,7 @@ impl Backtrace {
///
/// To forcibly capture a backtrace regardless of environment variables, use
/// the `Backtrace::force_capture` function.
#[stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
#[inline(never)] // want to make sure there's a frame here to remove
pub fn capture() -> Backtrace {
if !Backtrace::enabled() {
Expand All @@ -308,16 +308,16 @@ impl Backtrace {
/// Note that capturing a backtrace can be an expensive operation on some
/// platforms, so this should be used with caution in performance-sensitive
/// parts of code.
#[stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
#[inline(never)] // want to make sure there's a frame here to remove
pub fn force_capture() -> Backtrace {
Backtrace::create(Backtrace::force_capture as usize)
}

/// Forcibly captures a disabled backtrace, regardless of environment
/// variable configuration.
#[stable(feature = "backtrace", since = "1.65.0")]
#[rustc_const_stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
#[rustc_const_stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
pub const fn disabled() -> Backtrace {
Backtrace { inner: Inner::Disabled }
}
Expand Down Expand Up @@ -361,7 +361,7 @@ impl Backtrace {
/// Returns the status of this backtrace, indicating whether this backtrace
/// request was unsupported, disabled, or a stack trace was actually
/// captured.
#[stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
#[must_use]
pub fn status(&self) -> BacktraceStatus {
match self.inner {
Expand All @@ -381,7 +381,7 @@ impl<'a> Backtrace {
}
}

#[stable(feature = "backtrace", since = "1.65.0")]
#[stable(feature = "backtrace", since = "CURRENT_RUSTC_VERSION")]
impl fmt::Display for Backtrace {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
let capture = match &self.inner {
Expand Down
8 changes: 7 additions & 1 deletion src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ impl<'a> Builder<'a> {
test::CrateRustdocJsonTypes,
test::Linkcheck,
test::TierCheck,
test::ReplacePlaceholderTest,
test::Cargotest,
test::Cargo,
test::Rls,
Expand Down Expand Up @@ -746,7 +747,12 @@ impl<'a> Builder<'a> {
install::Src,
install::Rustc
),
Kind::Run => describe!(run::ExpandYamlAnchors, run::BuildManifest, run::BumpStage0),
Kind::Run => describe!(
run::ExpandYamlAnchors,
run::BuildManifest,
run::BumpStage0,
run::ReplaceVersionPlaceholder,
),
// These commands either don't use paths, or they're special-cased in Build::build()
Kind::Clean | Kind::Format | Kind::Setup => vec![],
}
Expand Down
22 changes: 22 additions & 0 deletions src/bootstrap/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,25 @@ impl Step for BumpStage0 {
builder.run(&mut cmd);
}
}

#[derive(Debug, PartialOrd, Ord, Copy, Clone, Hash, PartialEq, Eq)]
pub struct ReplaceVersionPlaceholder;

impl Step for ReplaceVersionPlaceholder {
type Output = ();
const ONLY_HOSTS: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.path("src/tools/replace-version-placeholder")
}

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(ReplaceVersionPlaceholder);
}

fn run(self, builder: &Builder<'_>) -> Self::Output {
let mut cmd = builder.tool_cmd(Tool::ReplaceVersionPlaceholder);
cmd.arg(&builder.src);
builder.run(&mut cmd);
}
}
37 changes: 37 additions & 0 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2527,6 +2527,43 @@ impl Step for TierCheck {
}
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct ReplacePlaceholderTest;

impl Step for ReplacePlaceholderTest {
type Output = ();
const ONLY_HOSTS: bool = true;
const DEFAULT: bool = true;

/// Ensure the version placeholder replacement tool builds
fn run(self, builder: &Builder<'_>) {
builder.info("build check for version replacement placeholder");

// Test the version placeholder replacement tool itself.
let bootstrap_host = builder.config.build;
let compiler = builder.compiler(0, bootstrap_host);
let cargo = tool::prepare_tool_cargo(
builder,
compiler,
Mode::ToolBootstrap,
bootstrap_host,
"test",
"src/tools/replace-version-placeholder",
SourceType::InTree,
&[],
);
try_run(builder, &mut cargo.into());
}

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.path("src/tools/replace-version-placeholder")
}

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(Self);
}
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct LintDocs {
pub compiler: Compiler,
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ bootstrap_tool!(
JsonDocCk, "src/tools/jsondocck", "jsondocck";
HtmlChecker, "src/tools/html-checker", "html-checker";
BumpStage0, "src/tools/bump-stage0", "bump-stage0";
ReplaceVersionPlaceholder, "src/tools/replace-version-placeholder", "replace-version-placeholder";
);

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
Expand Down
10 changes: 10 additions & 0 deletions src/tools/replace-version-placeholder/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[package]
name = "replace-version-placeholder"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
tidy = { path = "../tidy" }
walkdir = "2"
30 changes: 30 additions & 0 deletions src/tools/replace-version-placeholder/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use std::path::PathBuf;
use tidy::{t, walk};

pub const VERSION_PLACEHOLDER: &str = "CURRENT_RUSTC_VERSION";

fn main() {
let root_path: PathBuf = std::env::args_os().nth(1).expect("need path to root of repo").into();
let version_path = root_path.join("src").join("version");
let version_str = t!(std::fs::read_to_string(&version_path), version_path);
let version_str = version_str.trim();
walk::walk(
&root_path,
&mut |path| {
walk::filter_dirs(path)
// We exempt these as they require the placeholder
// for their operation
|| path.ends_with("compiler/rustc_passes/src/lib_features.rs")
|| path.ends_with("src/tools/tidy/src/features/version.rs")
|| path.ends_with("src/tools/replace-version-placeholder")
},
&mut |entry, contents| {
if !contents.contains(VERSION_PLACEHOLDER) {
return;
}
let new_contents = contents.replace(VERSION_PLACEHOLDER, version_str);
let path = entry.path();
t!(std::fs::write(&path, new_contents), path);
},
);
}
38 changes: 38 additions & 0 deletions src/tools/tidy/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,36 @@ pub fn check(
tidy_error!(bad, "Found {} features without a gate test.", gate_untested.len());
}

let (version, channel) = get_version_and_channel(src_path);

let all_features_iter = features
.iter()
.map(|feat| (feat, "lang"))
.chain(lib_features.iter().map(|feat| (feat, "lib")));
for ((feature_name, feature), kind) in all_features_iter {
let since = if let Some(since) = feature.since { since } else { continue };
if since > version && since != Version::CurrentPlaceholder {
tidy_error!(
bad,
"The stabilization version {since} of {kind} feature `{feature_name}` is newer than the current {version}"
);
}
if channel == "nightly" && since == version {
tidy_error!(
bad,
"The stabilization version {since} of {kind} feature `{feature_name}` is written out but should be {}",
version::VERSION_PLACEHOLDER
);
}
if channel != "nightly" && since == Version::CurrentPlaceholder {
tidy_error!(
bad,
"The placeholder use of {kind} feature `{feature_name}` is not allowed on the {} channel",
version::VERSION_PLACEHOLDER
);
}
}

if *bad {
return CollectedFeatures { lib: lib_features, lang: features };
}
Expand All @@ -195,6 +225,14 @@ pub fn check(
CollectedFeatures { lib: lib_features, lang: features }
}

fn get_version_and_channel(src_path: &Path) -> (Version, String) {
let version_str = t!(std::fs::read_to_string(src_path.join("version")));
let version_str = version_str.trim();
let version = t!(std::str::FromStr::from_str(&version_str).map_err(|e| format!("{e:?}")));
let channel_str = t!(std::fs::read_to_string(src_path.join("ci").join("channel")));
(version, channel_str.trim().to_owned())
}

fn format_features<'a>(
features: &'a Features,
family: &'a str,
Expand Down
Loading

0 comments on commit eaadb89

Please sign in to comment.