From 76c213bc535cbac30315a6a6aefaad1530216536 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Fri, 6 Apr 2018 20:31:40 +0200 Subject: [PATCH 01/18] target_feature 1.1 --- text/0000-target-feature-1.1.md | 174 ++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 text/0000-target-feature-1.1.md diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md new file mode 100644 index 00000000000..39acbc6d71e --- /dev/null +++ b/text/0000-target-feature-1.1.md @@ -0,0 +1,174 @@ +- Feature Name: `#[target_feature]` 1.1 +- Start Date: 2018-04-06 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +This RFC attempts to resolve some of the unresolved questions in [RFC 2045 +(`target_feature`)]. In particular, it allows: + +* specifying `#[target_feature]` functions without making them `unsafe fn` +* calling `#[target_feature]` functions in some contexts without `unsafe { }` blocks + +It achieves this by proposing three incremental steps that we can sequentially +make to improve the ergonomics and the safety of target-specific functionality +without adding run-time overhead. + +[RFC 2045 (`target_feature`)]: https://github.com/rust-lang/rfcs/pull/2045 + +# Motivation +[motivation]: #motivation + +> This is a brief recap of [RFC 2045 (`target_feature`)]. + +The `#[target_feature]` attribute allows Rust to generate machine code for a +function under the assumption that the hardware where the function will be +executed on supports some specific "features". + +If the hardware does not support the features, the machine code was generated +under assumptions that do not hold, and the behavior of executing the function +is undefined. + +[RFC 2045 (`target_feature`)] guarantees safety by requiring all +`#[target_feature]` functions to be `unsafe fn`, thus preventing them from being +called from safe code. That is, users have to open an `unsafe { }` block to call +these functions, and they have to manually ensure that their pre-conditions +hold - for example, that they will only be executed on the appropriate hardware +by doing run-time feature detection, or using conditional compilation. + +And that's it. That's all [RFC 2045 (`target_feature`)] had to say about this. +Back then, there were many other problems that needed to be solved for all of +this to be minially useful, and [RFC 2045 (`target_feature`)] dealt with those. + +However, the consensus back then was that this is far from ideal for many +reasons: + +* when calling `#[target_feature]` functions from other `#[target_feature]` + functions with the same features, the calls are currently still `unsafe` but + they are actually safe to call. +* making all `#[target_feature]` functions `unsafe fn`s and requiring `unsafe + {}` to call them everywhere hides other potential sources of `unsafe` within + these functions. Users get used to upholding `#[target_feature]`-related + pre-conditions, and other types of pre-conditions get glossed by. + +This RFC proposes concrete solutions for these two problems. + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + + +Currently, we require that `#[target_feature]` functions be declared as `unsafe +fn`. This RFC relaxes this restriction: + +* safe `#[target_feature]` functions can be called _without_ an `unsafe {}` +block _only_ from functions with the exact same set of `#[target_feature]`s. +Calling them from other contexts (other functions, static variable initializers, +etc.) requires opening an `unsafe {}` even though they are not marked as +`unsafe`: + +```rust +// Example 1: +#[target_feature = "sse2"] unsafe fn foo() { } // RFC2045 +#[target_feature = "sse2"] fn bar() { } // NEW + +// This function does not have the "sse2" target feature: +fn meow() { + foo(); // ERROR (unsafe block required) + unsafe { foo() }; // OK + bar(); // ERROR (meow is not sse2) + unsafe { bar() }; // OK +} + +#[target_feature = "sse2"] +fn bark() { + foo(); // ERROR (foo is unsafe: unsafe block required) + unsafe { foo() }; // OK + bar(); // OK (bark is sse2 and bar is safe) + unsafe { bar() }; // OK (as well - warning: unnecessary unsafe block) +} + +#[target_feature = "avx"] // avx != sse2 +fn moo() { + foo(); // ERROR (unsafe block required) + unsafe { foo() }; // OK + bar(); // ERROR (bark is not sse2) + unsafe { bar() }; // OK +} +``` + +> Note: while it is safe to call an SSE2 function from an AVX one, this would +> require specifying how features relate to each other in hierarchies. This +> would unnecessary complicate this RFC and can be done later once we agree on +> the fundamentals. + + +The `#[target_feature]` attribute continues to not be allowed on safe trait +method implementations: + +```rust +// Example 2: +trait Foo { fn foo(); } +struct Fooish(); +impl Foo for Fooish { + #[target_feature = "sse2"] fn foo() { } + // ^ ERROR: #[target_feature] on trait method impl requires + // unsafe fn but Foo::foo is safe + // (this is already an error per RFC2045) +} + +trait Bar { unsafe fn bar(); } +struct Barish(); +impl Bar for Barish { + #[target_feature = "sse2"] unsafe fn bar() { } // OK (RFC2045) +} +``` + +* safe `#[target_feature]` functions are not assignable to safe `fn` pointers. + + +``` +// Example 3 +#[target_feature] fn meow() {} + +static x: fn () -> () = meow; +// ^ ERROR: meow can only be assigned to unsafe fn pointers due to +// #[target_feature] but function pointer x with type fn()->() is safe. +static y: unsafe fn () -> () = meow as unsafe fn()->(); // OK +``` + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +This RFC proposes to changes to the language with respect to [RFC 2045 (`target_feature`)]: + +* safe `#[target_feature]` functions can be called _without_ an `unsafe {}` +block _only_ from functions with the exact same set of `#[target_feature]`s. +Calling them from other contexts (other functions, static variable initializers, +etc.) requires opening an `unsafe {}` even though they are not marked as +`unsafe` + +* safe `#[target_feature]` functions are not assignable to safe `fn` pointers. + +# Drawbacks +[drawbacks]: #drawbacks + +TBD. + +# Rationale and alternatives +[alternatives]: #alternatives + +TBD. + +# Prior art +[prior-art]: #prior-art + +[RFC2212 target feature unsafe](https://github.com/rust-lang/rfcs/pull/2212) +attempted to solve this problem. This RFC builds on the discussion that was +produced by that RFC and by many discussions in the `stdsimd` repo. + +# Unresolved questions +[unresolved]: #unresolved-questions + +TBD. From 54083aa88085767ed0629bcab074eb21bb4c00d9 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Fri, 6 Apr 2018 20:45:49 +0200 Subject: [PATCH 02/18] add perf implications --- text/0000-target-feature-1.1.md | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index 39acbc6d71e..c10c17e56a1 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -52,13 +52,16 @@ reasons: {}` to call them everywhere hides other potential sources of `unsafe` within these functions. Users get used to upholding `#[target_feature]`-related pre-conditions, and other types of pre-conditions get glossed by. +* `#[target_feature]` functions are not inlined across mismatching contexts, + which can have disastrous performance implications. Currently calling + `#[target_feature]` function from all contexts looks identical which makes it + easy for users to make these mistakes (which get reported often). -This RFC proposes concrete solutions for these two problems. +The solution proposed in this RFC solves these problems. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation - Currently, we require that `#[target_feature]` functions be declared as `unsafe fn`. This RFC relaxes this restriction: @@ -103,6 +106,18 @@ fn moo() { > would unnecessary complicate this RFC and can be done later once we agree on > the fundamentals. +This already solves all three issues mentioned in the motivation: + +* when calling `#[target_feature]` functions from other `#[target_feature]` + functions with the same features, we don't need `unsafe` code anymore. +* since `#[target_feature]` functions do not need to be `unsafe` anymore, + `#[target_feature]` functions that are marked with `unsafe` become more + visible, making it harder for users to oversee that there are other + pre-conditions that must be uphold. +* `#[target_feature]` function calls across mismatching contexts require + `unsafe`, making them more visible. This makes it easier to identify + calls-sites across which they cannot be inlined while making call-sites across + which they can be inlined more ergonomic to write. The `#[target_feature]` attribute continues to not be allowed on safe trait method implementations: From 2fec554e5fb428c86f5cac42f61c00bb232e9de7 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Fri, 6 Apr 2018 20:52:34 +0200 Subject: [PATCH 03/18] fmt --- text/0000-target-feature-1.1.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index c10c17e56a1..fadae9263d2 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -143,7 +143,7 @@ impl Bar for Barish { * safe `#[target_feature]` functions are not assignable to safe `fn` pointers. -``` +```rust // Example 3 #[target_feature] fn meow() {} From f9d49051d184bf32a36e4efa6c60036756d79ab6 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Fri, 6 Apr 2018 21:01:12 +0200 Subject: [PATCH 04/18] soundness --- text/0000-target-feature-1.1.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index fadae9263d2..9cb29dfc9b6 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -66,10 +66,10 @@ Currently, we require that `#[target_feature]` functions be declared as `unsafe fn`. This RFC relaxes this restriction: * safe `#[target_feature]` functions can be called _without_ an `unsafe {}` -block _only_ from functions with the exact same set of `#[target_feature]`s. -Calling them from other contexts (other functions, static variable initializers, -etc.) requires opening an `unsafe {}` even though they are not marked as -`unsafe`: +block _only_ from functions that have at least the exact same set of +`#[target_feature]`s. Calling them from other contexts (other functions, static +variable initializers, etc.) requires opening an `unsafe {}` even though they +are not marked as `unsafe`: ```rust // Example 1: @@ -106,7 +106,11 @@ fn moo() { > would unnecessary complicate this RFC and can be done later once we agree on > the fundamentals. -This already solves all three issues mentioned in the motivation: +First, this is still sound. The caller has a super-set of `#[target_features]` +of the callee. That is, the `#[target_feature]`-related pre-conditions of the +callee are uphold by the caller, therefore calling the callee is safe. + +This change already solves all three issues mentioned in the motivation: * when calling `#[target_feature]` functions from other `#[target_feature]` functions with the same features, we don't need `unsafe` code anymore. From 39e03e3da8ca9c47cffed4803550823860685301 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Fri, 6 Apr 2018 21:02:32 +0200 Subject: [PATCH 05/18] fmt --- text/0000-target-feature-1.1.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index 9cb29dfc9b6..cde4a946370 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -112,9 +112,9 @@ callee are uphold by the caller, therefore calling the callee is safe. This change already solves all three issues mentioned in the motivation: -* when calling `#[target_feature]` functions from other `#[target_feature]` +* When calling `#[target_feature]` functions from other `#[target_feature]` functions with the same features, we don't need `unsafe` code anymore. -* since `#[target_feature]` functions do not need to be `unsafe` anymore, +* Since `#[target_feature]` functions do not need to be `unsafe` anymore, `#[target_feature]` functions that are marked with `unsafe` become more visible, making it harder for users to oversee that there are other pre-conditions that must be uphold. From aa79d09670dda105e9775411caa31bc2d06b2be1 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Wed, 11 Apr 2018 10:30:44 +0200 Subject: [PATCH 06/18] fix bug in reference; add unresolved question about disabling features --- text/0000-target-feature-1.1.md | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index cde4a946370..637e53a32cf 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -163,10 +163,10 @@ static y: unsafe fn () -> () = meow as unsafe fn()->(); // OK This RFC proposes to changes to the language with respect to [RFC 2045 (`target_feature`)]: * safe `#[target_feature]` functions can be called _without_ an `unsafe {}` -block _only_ from functions with the exact same set of `#[target_feature]`s. -Calling them from other contexts (other functions, static variable initializers, -etc.) requires opening an `unsafe {}` even though they are not marked as -`unsafe` +block _only_ from functions that have at least the exact same set of +`#[target_feature]`s. Calling them from other contexts (other functions, static +variable initializers, etc.) requires opening an `unsafe {}` even though they +are not marked as `unsafe` * safe `#[target_feature]` functions are not assignable to safe `fn` pointers. @@ -190,4 +190,25 @@ produced by that RFC and by many discussions in the `stdsimd` repo. # Unresolved questions [unresolved]: #unresolved-questions -TBD. +## Negative features + +[RFC 2045 (`target_feature`)] introduced the `#[target_feature(enable = "x")]` +syntax to allow introducing negative features in future RFCs in the form of +`#[target_feature(disable = "y")]`. Since these have not been introduced yet we +can only speculate about how they would interact with the extensions proposed in +this RFC but we probably can make the following work in some form: + +```rust +// #[target_feature(enable = "sse")] +fn foo() {} + +#[target_feature(disable = "sse")] +fn bar() { + foo(); // ERROR: (bar is not sse) + unsafe { foo() }; // OK +} + +fn baz() { + bar(); // OK +} +``` From b73fc95e60d6635e012b1eaf52bdd544f1833596 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Wed, 11 Apr 2018 11:05:03 +0200 Subject: [PATCH 07/18] effect system --- text/0000-target-feature-1.1.md | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index 637e53a32cf..3c8c50200c9 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -173,12 +173,19 @@ are not marked as `unsafe` # Drawbacks [drawbacks]: #drawbacks -TBD. +This RFC extends the typing rules for `#[target_feature]`, which might +unnecessarily complicate future language features like an effect system. # Rationale and alternatives [alternatives]: #alternatives -TBD. +The alternative here is to make `#[target_feature]` an effect/restriction. Since +`#[target_feature]` is already on its path to stabilization and its required to +make SIMD in Rust minimally useful, not stabilizing `#[target_feature]` at this +point is not an option unless we are willing to delay SIMD until an effect +system is stabilized. Until then, `#[target_feature]` and this RFC solve real +problems and provide another use-case that such an effect system will need to +enable to be useful for Rust programmers. # Prior art [prior-art]: #prior-art @@ -212,3 +219,21 @@ fn baz() { bar(); // OK } ``` + +## Effect system + +It is unclear how `#[target_feature]` would interact with an effect system for +Rust like the one being tracked +[here](https://github.com/Centril/rfc-effects/issues) and discussed in +[RFC2237](https://github.com/rust-lang/rfcs/pull/2237). + +In particular, it is unclear how the typing rules being proposed here would be +covered by such an effect system, and whether such system would support +attributes in effect/restriction position. + +Such an effect-system might need to introduce first-class target-features into +the language (beyond just a simple attribute) which could lead to the +deprecation of the `#[target_feature]` attribute. + +It is also unclear how any of this interacts with effect-polymorphism at this +point. From 96c3abbf9021551f3cf4fe9a58bf2dfae64b2a78 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Mon, 23 Apr 2018 17:04:21 +0200 Subject: [PATCH 08/18] update effects text --- text/0000-target-feature-1.1.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index 3c8c50200c9..ede813e3363 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -179,13 +179,14 @@ unnecessarily complicate future language features like an effect system. # Rationale and alternatives [alternatives]: #alternatives -The alternative here is to make `#[target_feature]` an effect/restriction. Since -`#[target_feature]` is already on its path to stabilization and its required to -make SIMD in Rust minimally useful, not stabilizing `#[target_feature]` at this -point is not an option unless we are willing to delay SIMD until an effect -system is stabilized. Until then, `#[target_feature]` and this RFC solve real -problems and provide another use-case that such an effect system will need to -enable to be useful for Rust programmers. +Since `#[target_feature]` are effects or restrictions (depending on whether we +`enable` or `disable` them), the alternative would be to integrate them with an +effect system. Since `#[target_feature]` is already on its path to stabilization +and its required to make SIMD in Rust minimally useful, not stabilizing +`#[target_feature]` at this point is not an option unless we are willing to +delay SIMD until an effect system is stabilized. Until then, `#[target_feature]` +and this RFC solve real problems and provide another use-case that such an +effect system will need to enable to be useful for Rust programmers. # Prior art [prior-art]: #prior-art From 48f4616c3d4d4ef7642468a10485a7d2fc9f1cb2 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Mon, 23 Apr 2018 17:05:47 +0200 Subject: [PATCH 09/18] clarify hierarchies --- text/0000-target-feature-1.1.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index ede813e3363..8f58c5599e8 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -101,10 +101,11 @@ fn moo() { } ``` -> Note: while it is safe to call an SSE2 function from an AVX one, this would -> require specifying how features relate to each other in hierarchies. This -> would unnecessary complicate this RFC and can be done later once we agree on -> the fundamentals. +> Note: while it is safe to call an SSE2 function from _some_ AVX functions, +> this would require specifying how features relate to each other in +> hierarchies. It is unclear whether those hierarchies actually exist, but +> adding them to this RFC wouldunnecessary complicate it and can be done +> later or in parallel to this one, once we agree on the fundamentals. First, this is still sound. The caller has a super-set of `#[target_features]` of the callee. That is, the `#[target_feature]`-related pre-conditions of the From c82ebf92259abacce975e1fe69d2c1ccda029cd7 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Tue, 16 Jul 2019 11:39:02 +0200 Subject: [PATCH 10/18] Update text/0000-target-feature-1.1.md --- text/0000-target-feature-1.1.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index 8f58c5599e8..c497cb1cb0b 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -96,7 +96,7 @@ fn bark() { fn moo() { foo(); // ERROR (unsafe block required) unsafe { foo() }; // OK - bar(); // ERROR (bark is not sse2) + bar(); // ERROR (bar is not sse2) unsafe { bar() }; // OK } ``` From cf7f51393212bc70e51eb2cf3d2158cb100e9342 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Tue, 16 Jul 2019 11:40:52 +0200 Subject: [PATCH 11/18] Update text/0000-target-feature-1.1.md --- text/0000-target-feature-1.1.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index c497cb1cb0b..0a986f2d66d 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -96,7 +96,7 @@ fn bark() { fn moo() { foo(); // ERROR (unsafe block required) unsafe { foo() }; // OK - bar(); // ERROR (bar is not sse2) + bar(); // ERROR (moo is not sse2 but bar requires it) unsafe { bar() }; // OK } ``` From f2894b096876a91890ddd1d32b2edda17f3c2ced Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Wed, 27 Nov 2019 16:16:03 +0100 Subject: [PATCH 12/18] Reword Co-Authored-By: Mazdak Farrokhzad --- text/0000-target-feature-1.1.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index 0a986f2d66d..fe63d0a0c49 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -40,7 +40,7 @@ by doing run-time feature detection, or using conditional compilation. And that's it. That's all [RFC 2045 (`target_feature`)] had to say about this. Back then, there were many other problems that needed to be solved for all of -this to be minially useful, and [RFC 2045 (`target_feature`)] dealt with those. +this to be minimally useful, and [RFC 2045 (`target_feature`)] dealt with those. However, the consensus back then was that this is far from ideal for many reasons: From 43153714691fb2c81fe4c669e8f75da7677850cb Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Wed, 27 Nov 2019 16:16:47 +0100 Subject: [PATCH 13/18] Reword Co-Authored-By: Mazdak Farrokhzad --- text/0000-target-feature-1.1.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index fe63d0a0c49..cfafd74027d 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -104,7 +104,7 @@ fn moo() { > Note: while it is safe to call an SSE2 function from _some_ AVX functions, > this would require specifying how features relate to each other in > hierarchies. It is unclear whether those hierarchies actually exist, but -> adding them to this RFC wouldunnecessary complicate it and can be done +> adding them to this RFC would unnecessarily complicate it and can be done > later or in parallel to this one, once we agree on the fundamentals. First, this is still sound. The caller has a super-set of `#[target_features]` From 578c30d4219388a916a96c9b2614d209529146db Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Wed, 27 Nov 2019 16:34:10 +0100 Subject: [PATCH 14/18] Update syntax --- text/0000-target-feature-1.1.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index cfafd74027d..25c1966a198 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -73,8 +73,8 @@ are not marked as `unsafe`: ```rust // Example 1: -#[target_feature = "sse2"] unsafe fn foo() { } // RFC2045 -#[target_feature = "sse2"] fn bar() { } // NEW +#[target_feature(enable = "sse2")] unsafe fn foo() { } // RFC2045 +#[target_feature(enable = "sse2")] fn bar() { } // NEW // This function does not have the "sse2" target feature: fn meow() { @@ -84,7 +84,7 @@ fn meow() { unsafe { bar() }; // OK } -#[target_feature = "sse2"] +#[target_feature(enable = "sse2")] fn bark() { foo(); // ERROR (foo is unsafe: unsafe block required) unsafe { foo() }; // OK @@ -92,7 +92,7 @@ fn bark() { unsafe { bar() }; // OK (as well - warning: unnecessary unsafe block) } -#[target_feature = "avx"] // avx != sse2 +#[target_feature(enable = "avx")] // avx != sse2 fn moo() { foo(); // ERROR (unsafe block required) unsafe { foo() }; // OK @@ -132,7 +132,7 @@ method implementations: trait Foo { fn foo(); } struct Fooish(); impl Foo for Fooish { - #[target_feature = "sse2"] fn foo() { } + #[target_feature(enable = "sse2")] fn foo() { } // ^ ERROR: #[target_feature] on trait method impl requires // unsafe fn but Foo::foo is safe // (this is already an error per RFC2045) @@ -141,7 +141,7 @@ impl Foo for Fooish { trait Bar { unsafe fn bar(); } struct Barish(); impl Bar for Barish { - #[target_feature = "sse2"] unsafe fn bar() { } // OK (RFC2045) + #[target_feature(enable = "sse2")] unsafe fn bar() { } // OK (RFC2045) } ``` @@ -150,7 +150,7 @@ impl Bar for Barish { ```rust // Example 3 -#[target_feature] fn meow() {} +#[target_feature(enable = "avx")] fn meow() {} static x: fn () -> () = meow; // ^ ERROR: meow can only be assigned to unsafe fn pointers due to From 6e5ce9d8cf123122e9a8a142b887fa8fbb5d6b5a Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Wed, 27 Nov 2019 16:40:13 +0100 Subject: [PATCH 15/18] Document potential effect polymorphism syntax --- text/0000-target-feature-1.1.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index 25c1966a198..a77f4633d42 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -238,4 +238,9 @@ the language (beyond just a simple attribute) which could lead to the deprecation of the `#[target_feature]` attribute. It is also unclear how any of this interacts with effect-polymorphism at this -point. +point, but we could support something like `impl const Trait` and `T: const Trait`: + +```rust +impl #[target_feature(enable = "...")] Trait for Type { ... } +fn foo(...) { ...} +``` From a81b5a6fd6db9e63d842aee34c3898150a5f8681 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Wed, 27 Nov 2019 17:16:49 +0100 Subject: [PATCH 16/18] Mention that target_feature is already allowed on inherent methods and this RFC does not change that --- text/0000-target-feature-1.1.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index a77f4633d42..a4cf9fcf8c0 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -124,6 +124,9 @@ This change already solves all three issues mentioned in the motivation: calls-sites across which they cannot be inlined while making call-sites across which they can be inlined more ergonomic to write. +The `#[target_feature]` attribute continues to be allowed on inherent methods - +this RFC does not change that. + The `#[target_feature]` attribute continues to not be allowed on safe trait method implementations: @@ -182,12 +185,7 @@ unnecessarily complicate future language features like an effect system. Since `#[target_feature]` are effects or restrictions (depending on whether we `enable` or `disable` them), the alternative would be to integrate them with an -effect system. Since `#[target_feature]` is already on its path to stabilization -and its required to make SIMD in Rust minimally useful, not stabilizing -`#[target_feature]` at this point is not an option unless we are willing to -delay SIMD until an effect system is stabilized. Until then, `#[target_feature]` -and this RFC solve real problems and provide another use-case that such an -effect system will need to enable to be useful for Rust programmers. +effect system. # Prior art [prior-art]: #prior-art From 6b9a16f56a2441811d1f2d7e6bbeedef7f049b09 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Wed, 27 Nov 2019 17:18:18 +0100 Subject: [PATCH 17/18] Clarify why target_feature is not allowed on the implementation of safe trait methods. --- text/0000-target-feature-1.1.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index a4cf9fcf8c0..e1309b08108 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -128,7 +128,8 @@ The `#[target_feature]` attribute continues to be allowed on inherent methods - this RFC does not change that. The `#[target_feature]` attribute continues to not be allowed on safe trait -method implementations: +method implementations because that would require an `unsafe` trait method +declaration: ```rust // Example 2: From 71b9069938ba6f0d4a75427f19d7ff859d5fdcb6 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Wed, 27 Nov 2019 17:23:21 +0100 Subject: [PATCH 18/18] Clarify when the impl target_feature Trait can work --- text/0000-target-feature-1.1.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/text/0000-target-feature-1.1.md b/text/0000-target-feature-1.1.md index e1309b08108..c8a5398dd89 100644 --- a/text/0000-target-feature-1.1.md +++ b/text/0000-target-feature-1.1.md @@ -237,9 +237,14 @@ the language (beyond just a simple attribute) which could lead to the deprecation of the `#[target_feature]` attribute. It is also unclear how any of this interacts with effect-polymorphism at this -point, but we could support something like `impl const Trait` and `T: const Trait`: +point, but we could _maybe_ support something like `impl const Trait` and `T: +const Trait`: ```rust impl #[target_feature(enable = "...")] Trait for Type { ... } fn foo(...) { ...} ``` + +if all trait methods are `unsafe`; otherwise they can't have the +`#[target_feature]` attribute. +