Skip to content

Commit

Permalink
Replace version_check dependency with own version parsing code
Browse files Browse the repository at this point in the history
This gives compiler maintainers a better degree of control
over how the version gets parsed and is a good way to ensure
that there are no changes of behaviour in the future.

Also, issue a warning if the version is invalid instead of erroring
so that we stay forwards compatible with possible future changes
of the versioning scheme.

Last, this improves the present test a little.
  • Loading branch information
est31 committed Jan 24, 2021
1 parent 202720b commit 14aa12f
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 36 deletions.
1 change: 0 additions & 1 deletion Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3568,7 +3568,6 @@ dependencies = [
"rustc_serialize",
"rustc_session",
"rustc_span",
"version_check",
]

[[package]]
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_attr/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,3 @@ rustc_lexer = { path = "../rustc_lexer" }
rustc_macros = { path = "../rustc_macros" }
rustc_session = { path = "../rustc_session" }
rustc_ast = { path = "../rustc_ast" }
version_check = "0.9"
32 changes: 28 additions & 4 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use rustc_session::Session;
use rustc_span::hygiene::Transparency;
use rustc_span::{symbol::sym, symbol::Symbol, Span};
use std::num::NonZeroU32;
use version_check::Version;

pub fn is_builtin_attr(attr: &Attribute) -> bool {
attr.is_doc_comment() || attr.ident().filter(|ident| is_builtin_attr_name(ident.name)).is_some()
Expand Down Expand Up @@ -526,6 +525,26 @@ fn gate_cfg(gated_cfg: &GatedCfg, cfg_span: Span, sess: &ParseSess, features: &F
}
}

#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
struct Version {
major: u16,
minor: u16,
patch: u16,
}

fn parse_version(s: &str, allow_appendix: bool) -> Option<Version> {
let mut components = s.split('-');
let d = components.next()?;
if !allow_appendix && components.next().is_some() {
return None;
}
let mut digits = d.splitn(3, '.');
let major = digits.next()?.parse().ok()?;
let minor = digits.next()?.parse().ok()?;
let patch = digits.next().unwrap_or("0").parse().ok()?;
Some(Version { major, minor, patch })
}

/// Evaluate a cfg-like condition (with `any` and `all`), using `eval` to
/// evaluate individual items.
pub fn eval_condition(
Expand Down Expand Up @@ -555,16 +574,21 @@ pub fn eval_condition(
return false;
}
};
let min_version = match Version::parse(&min_version.as_str()) {
let min_version = match parse_version(&min_version.as_str(), false) {
Some(ver) => ver,
None => {
sess.span_diagnostic.struct_span_err(*span, "invalid version literal").emit();
sess.span_diagnostic
.struct_span_warn(
*span,
"unknown version literal format, assuming it refers to a future version",
)
.emit();
return false;
}
};
let channel = env!("CFG_RELEASE_CHANNEL");
let nightly = channel == "nightly" || channel == "dev";
let rustc_version = Version::parse(env!("CFG_RELEASE")).unwrap();
let rustc_version = parse_version(env!("CFG_RELEASE"), true).unwrap();

// See https://github.com/rust-lang/rust/issues/64796#issuecomment-625474439 for details
if nightly { rustc_version > min_version } else { rustc_version >= min_version }
Expand Down
30 changes: 19 additions & 11 deletions src/test/ui/feature-gates/feature-gate-cfg-version.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
#[cfg(version(42))] //~ ERROR: expected a version literal
//~^ ERROR `cfg(version)` is experimental and subject to change
fn foo() {}
#[cfg(version(1.20))] //~ ERROR: expected a version literal
//~^ ERROR `cfg(version)` is experimental and subject to change
fn foo() -> bool { true }
#[cfg(version("1.44"))]
//~^ ERROR `cfg(version)` is experimental and subject to change
fn foo() -> bool { true }
Expand All @@ -11,30 +17,32 @@ fn bar() -> bool { false }
#[cfg(version(false))] //~ ERROR: expected a version literal
//~^ ERROR `cfg(version)` is experimental and subject to change
fn bar() -> bool { false }
#[cfg(version("foo"))] //~ ERROR: invalid version literal
#[cfg(version("foo"))] //~ WARNING: unknown version literal format
//~^ ERROR `cfg(version)` is experimental and subject to change
fn bar() -> bool { false }
#[cfg(version("999"))]
#[cfg(version("999"))] //~ WARNING: unknown version literal format
//~^ ERROR `cfg(version)` is experimental and subject to change
fn bar() -> bool { false }
#[cfg(version("-1"))] //~ ERROR: invalid version literal
#[cfg(version("-1"))] //~ WARNING: unknown version literal format
//~^ ERROR `cfg(version)` is experimental and subject to change
fn bar() -> bool { false }
#[cfg(version("65536"))] //~ ERROR: invalid version literal
#[cfg(version("65536"))] //~ WARNING: unknown version literal format
//~^ ERROR `cfg(version)` is experimental and subject to change
fn bar() -> bool { false }
#[cfg(version("0"))]
#[cfg(version("0"))] //~ WARNING: unknown version literal format
//~^ ERROR `cfg(version)` is experimental and subject to change
fn bar() -> bool { true }

#[cfg(version("1.65536.2"))]
#[cfg(version("1.0"))]
//~^ ERROR `cfg(version)` is experimental and subject to change
fn bar() -> bool { true }
#[cfg(version("1.65536.2"))] //~ WARNING: unknown version literal format
//~^ ERROR `cfg(version)` is experimental and subject to change
fn bar() -> bool { false }
#[cfg(version("1.20.0-stable"))] //~ WARNING: unknown version literal format
//~^ ERROR `cfg(version)` is experimental and subject to change
fn version_check_bug() {}
fn bar() {}

fn main() {
// This should fail but due to a bug in version_check `1.65536.2` is interpreted as `1.2`.
// See https://github.com/SergioBenitez/version_check/issues/11
version_check_bug();
assert!(foo());
assert!(bar());
assert!(cfg!(version("1.42"))); //~ ERROR `cfg(version)` is experimental and subject to change
Expand Down
110 changes: 91 additions & 19 deletions src/test/ui/feature-gates/feature-gate-cfg-version.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,44 @@
error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:1:7
|
LL | #[cfg(version(42))]
| ^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error: expected a version literal
--> $DIR/feature-gate-cfg-version.rs:1:15
|
LL | #[cfg(version(42))]
| ^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:4:7
|
LL | #[cfg(version(1.20))]
| ^^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error: expected a version literal
--> $DIR/feature-gate-cfg-version.rs:4:15
|
LL | #[cfg(version(1.20))]
| ^^^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:7:7
|
LL | #[cfg(version("1.44"))]
| ^^^^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:4:11
--> $DIR/feature-gate-cfg-version.rs:10:11
|
LL | #[cfg(not(version("1.44")))]
| ^^^^^^^^^^^^^^^
Expand All @@ -17,7 +47,7 @@ LL | #[cfg(not(version("1.44")))]
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:8:7
--> $DIR/feature-gate-cfg-version.rs:14:7
|
LL | #[cfg(version("1.43", "1.44", "1.45"))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -26,13 +56,13 @@ LL | #[cfg(version("1.43", "1.44", "1.45"))]
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error: expected single version literal
--> $DIR/feature-gate-cfg-version.rs:8:7
--> $DIR/feature-gate-cfg-version.rs:14:7
|
LL | #[cfg(version("1.43", "1.44", "1.45"))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:11:7
--> $DIR/feature-gate-cfg-version.rs:17:7
|
LL | #[cfg(version(false))]
| ^^^^^^^^^^^^^^
Expand All @@ -41,92 +71,134 @@ LL | #[cfg(version(false))]
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error: expected a version literal
--> $DIR/feature-gate-cfg-version.rs:11:15
--> $DIR/feature-gate-cfg-version.rs:17:15
|
LL | #[cfg(version(false))]
| ^^^^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:14:7
--> $DIR/feature-gate-cfg-version.rs:20:7
|
LL | #[cfg(version("foo"))]
| ^^^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error: invalid version literal
--> $DIR/feature-gate-cfg-version.rs:14:15
warning: unknown version literal format, assuming it refers to a future version
--> $DIR/feature-gate-cfg-version.rs:20:15
|
LL | #[cfg(version("foo"))]
| ^^^^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:17:7
--> $DIR/feature-gate-cfg-version.rs:23:7
|
LL | #[cfg(version("999"))]
| ^^^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

warning: unknown version literal format, assuming it refers to a future version
--> $DIR/feature-gate-cfg-version.rs:23:15
|
LL | #[cfg(version("999"))]
| ^^^^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:20:7
--> $DIR/feature-gate-cfg-version.rs:26:7
|
LL | #[cfg(version("-1"))]
| ^^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error: invalid version literal
--> $DIR/feature-gate-cfg-version.rs:20:15
warning: unknown version literal format, assuming it refers to a future version
--> $DIR/feature-gate-cfg-version.rs:26:15
|
LL | #[cfg(version("-1"))]
| ^^^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:23:7
--> $DIR/feature-gate-cfg-version.rs:29:7
|
LL | #[cfg(version("65536"))]
| ^^^^^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error: invalid version literal
--> $DIR/feature-gate-cfg-version.rs:23:15
warning: unknown version literal format, assuming it refers to a future version
--> $DIR/feature-gate-cfg-version.rs:29:15
|
LL | #[cfg(version("65536"))]
| ^^^^^^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:26:7
--> $DIR/feature-gate-cfg-version.rs:32:7
|
LL | #[cfg(version("0"))]
| ^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

warning: unknown version literal format, assuming it refers to a future version
--> $DIR/feature-gate-cfg-version.rs:32:15
|
LL | #[cfg(version("0"))]
| ^^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:35:7
|
LL | #[cfg(version("1.0"))]
| ^^^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:30:7
--> $DIR/feature-gate-cfg-version.rs:38:7
|
LL | #[cfg(version("1.65536.2"))]
| ^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

warning: unknown version literal format, assuming it refers to a future version
--> $DIR/feature-gate-cfg-version.rs:38:15
|
LL | #[cfg(version("1.65536.2"))]
| ^^^^^^^^^^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:41:7
|
LL | #[cfg(version("1.20.0-stable"))]
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

warning: unknown version literal format, assuming it refers to a future version
--> $DIR/feature-gate-cfg-version.rs:41:15
|
LL | #[cfg(version("1.20.0-stable"))]
| ^^^^^^^^^^^^^^^

error[E0658]: `cfg(version)` is experimental and subject to change
--> $DIR/feature-gate-cfg-version.rs:40:18
--> $DIR/feature-gate-cfg-version.rs:48:18
|
LL | assert!(cfg!(version("1.42")));
| ^^^^^^^^^^^^^^^
|
= note: see issue #64796 <https://github.com/rust-lang/rust/issues/64796> for more information
= help: add `#![feature(cfg_version)]` to the crate attributes to enable

error: aborting due to 16 previous errors
error: aborting due to 19 previous errors; 7 warnings emitted

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

0 comments on commit 14aa12f

Please sign in to comment.