about summary refs log tree commit diff
path: root/src/test/ui
AgeCommit message (Collapse)AuthorLines
2022-02-20Improve `unused_unsafe` lintFrank Steffahn-36/+2907
Main motivation: Fixes some issues with the current behavior. This PR is more-or-less completely re-implementing the unused_unsafe lint; it’s also only done in the MIR-version of the lint, the set of tests for the `-Zthir-unsafeck` version no longer succeeds (and is thus disabled, see `lint-unused-unsafe.rs`). On current nightly, ```rs unsafe fn unsf() {} fn inner_ignored() { unsafe { #[allow(unused_unsafe)] unsafe { unsf() } } } ``` doesn’t create any warnings. This situation is not unrealistic to come by, the inner `unsafe` block could e.g. come from a macro. Actually, this PR even includes removal of one unused `unsafe` in the standard library that was missed in a similar situation. (The inner `unsafe` coming from an external macro hides the warning, too.) The reason behind this problem is how the check currently works: * While generating MIR, it already skips nested unsafe blocks (i.e. unsafe nested in other unsafe) so that the inner one is always the one considered unused * To differentiate the cases of no unsafe operations inside the `unsafe` vs. a surrounding `unsafe` block, there’s some ad-hoc magic walking up the HIR to look for surrounding used `unsafe` blocks. There’s a lot of problems with this approach besides the one presented above. E.g. the MIR-building uses checks for `unsafe_op_in_unsafe_fn` lint to decide early whether or not `unsafe` blocks in an `unsafe fn` are redundant and ought to be removed. ```rs unsafe fn granular_disallow_op_in_unsafe_fn() { unsafe { #[deny(unsafe_op_in_unsafe_fn)] { unsf(); } } } ``` ``` error: call to unsafe function is unsafe and requires unsafe block (error E0133) --> src/main.rs:13:13 | 13 | unsf(); | ^^^^^^ call to unsafe function | note: the lint level is defined here --> src/main.rs:11:16 | 11 | #[deny(unsafe_op_in_unsafe_fn)] | ^^^^^^^^^^^^^^^^^^^^^^ = note: consult the function's documentation for information on how to avoid undefined behavior warning: unnecessary `unsafe` block --> src/main.rs:10:5 | 9 | unsafe fn granular_disallow_op_in_unsafe_fn() { | --------------------------------------------- because it's nested under this `unsafe` fn 10 | unsafe { | ^^^^^^ unnecessary `unsafe` block | = note: `#[warn(unused_unsafe)]` on by default ``` Here, the intermediate `unsafe` was ignored, even though it contains a unsafe operation that is not allowed to happen in an `unsafe fn` without an additional `unsafe` block. Also closures were problematic and the workaround/algorithms used on current nightly didn’t work properly. (I skipped trying to fully understand what it was supposed to do, because this PR uses a completely different approach.) ```rs fn nested() { unsafe { unsafe { unsf() } } } ``` ``` warning: unnecessary `unsafe` block --> src/main.rs:10:9 | 9 | unsafe { | ------ because it's nested under this `unsafe` block 10 | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block | = note: `#[warn(unused_unsafe)]` on by default ``` vs ```rs fn nested() { let _ = || unsafe { let _ = || unsafe { unsf() }; }; } ``` ``` warning: unnecessary `unsafe` block --> src/main.rs:9:16 | 9 | let _ = || unsafe { | ^^^^^^ unnecessary `unsafe` block | = note: `#[warn(unused_unsafe)]` on by default warning: unnecessary `unsafe` block --> src/main.rs:10:20 | 10 | let _ = || unsafe { unsf() }; | ^^^^^^ unnecessary `unsafe` block ``` *note that this warning kind-of suggests that **both** unsafe blocks are redundant* -------------------------------------------------------------------------------- I also dislike the fact that it always suggests keeping the outermost `unsafe`. E.g. for ```rs fn granularity() { unsafe { unsafe { unsf() } unsafe { unsf() } unsafe { unsf() } } } ``` I prefer if `rustc` suggests removing the more-course outer-level `unsafe` instead of the fine-grained inner `unsafe` blocks, which it currently does on nightly: ``` warning: unnecessary `unsafe` block --> src/main.rs:10:9 | 9 | unsafe { | ------ because it's nested under this `unsafe` block 10 | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block | = note: `#[warn(unused_unsafe)]` on by default warning: unnecessary `unsafe` block --> src/main.rs:11:9 | 9 | unsafe { | ------ because it's nested under this `unsafe` block 10 | unsafe { unsf() } 11 | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block warning: unnecessary `unsafe` block --> src/main.rs:12:9 | 9 | unsafe { | ------ because it's nested under this `unsafe` block ... 12 | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block ``` -------------------------------------------------------------------------------- Needless to say, this PR addresses all these points. For context, as far as my understanding goes, the main advantage of skipping inner unsafe blocks was that a test case like ```rs fn top_level_used() { unsafe { unsf(); unsafe { unsf() } unsafe { unsf() } unsafe { unsf() } } } ``` should generate some warning because there’s redundant nested `unsafe`, however every single `unsafe` block _does_ contain some statement that uses it. Of course this PR doesn’t aim change the warnings on this kind of code example, because the current behavior, warning on all the inner `unsafe` blocks, makes sense in this case. As mentioned, during MIR building all the unsafe blocks *are* kept now, and usage is attributed to them. The way to still generate a warning like ``` warning: unnecessary `unsafe` block --> src/main.rs:11:9 | 9 | unsafe { | ------ because it's nested under this `unsafe` block 10 | unsf(); 11 | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block | = note: `#[warn(unused_unsafe)]` on by default warning: unnecessary `unsafe` block --> src/main.rs:12:9 | 9 | unsafe { | ------ because it's nested under this `unsafe` block ... 12 | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block warning: unnecessary `unsafe` block --> src/main.rs:13:9 | 9 | unsafe { | ------ because it's nested under this `unsafe` block ... 13 | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block ``` in this case is by emitting a `unused_unsafe` warning for all of the `unsafe` blocks that are _within a **used** unsafe block_. The previous code had a little HIR traversal already anyways to collect a set of all the unsafe blocks (in order to afterwards determine which ones are unused afterwards). This PR uses such a traversal to do additional things including logic like _always_ warn for an `unsafe` block that’s inside of another **used** unsafe block. The traversal is expanded to include nested closures in the same go, this simplifies a lot of things. The whole logic around `unsafe_op_in_unsafe_fn` is a little complicated, there’s some test cases of corner-cases in this PR. (The implementation involves differentiating between whether a used unsafe block was used exclusively by operations where `allow(unsafe_op_in_unsafe_fn)` was active.) The main goal was to make sure that code should compile successfully if all the `unused_unsafe`-warnings are addressed _simultaneously_ (by removing the respective `unsafe` blocks) no matter how complicated the patterns of `unsafe_op_in_unsafe_fn` being disallowed and allowed throughout the function are. -------------------------------------------------------------------------------- One noteworthy design decision I took here: An `unsafe` block with `allow(unused_unsafe)` **is considered used** for the purposes of linting about redundant contained unsafe blocks. So while ```rs fn granularity() { unsafe { //~ ERROR: unnecessary `unsafe` block unsafe { unsf() } unsafe { unsf() } unsafe { unsf() } } } ``` warns for the outer `unsafe` block, ```rs fn top_level_ignored() { #[allow(unused_unsafe)] unsafe { #[deny(unused_unsafe)] { unsafe { unsf() } //~ ERROR: unnecessary `unsafe` block unsafe { unsf() } //~ ERROR: unnecessary `unsafe` block unsafe { unsf() } //~ ERROR: unnecessary `unsafe` block } } } ``` warns on the inner ones.
2022-02-20Gracefully handle non-UTF-8 string slices when pretty printingTomasz Miąsko-16/+0
2022-02-19Bless upMichael Goulet-48/+6
2022-02-19No branch protection metadata unless enabledSimonas Kazlauskas-1/+18
Even if we emit metadata disabling branch protection, this metadata may conflict with other modules (e.g. during LTO) that have different branch protection metadata set. This is an unstable flag and feature, so ideally the flag not being specified should act as if the feature wasn't implemented in the first place. Additionally this PR also ensures we emit an error if `-Zbranch-protection` is set on targets other than the supported aarch64. For now the error is being output from codegen, but ideally it should be moved to earlier in the pipeline before stabilization.
2022-02-19Rollup merge of #93990 - lcnr:pre-89862-cleanup, r=estebankMatthias Krüger-4/+4
pre #89862 cleanup changes used in #89862 which can be landed without the rest of this PR being finished. r? `@estebank`
2022-02-19Rollup merge of #93658 - cchiw:issue-77443-fix, r=joshtriplettMatthias Krüger-36/+2
Stabilize `#[cfg(panic = "...")]` [Stabilization PR](https://rustc-dev-guide.rust-lang.org/stabilization_guide.html#stabilization-pr) for #77443
2022-02-18Rollup merge of #93953 - jackh726:known_bug, r=Mark-SimulacrumMatthias Krüger-27/+32
Add the `known-bug` test directive, use it, and do some cleanup cc rust-lang/compiler-team#476 Now tests can be annotated with `known-bug`, which should indicate that the test *should* pass (or at least that the current output is a bug). Adding it relaxes the requirement to add error annotations to the test (though it is still allowed). In the future, this could be extended with further relaxations - with the goal to make adding these tests need minimal effort. I've used this attribute for the GAT tests added in #93757. Finally, I've also cleaned up `header.rs` in compiletest a bit, by extracting out a bit of common logic. I've also split out some of the directives into their own consts. This removes a lot of very similar functions from `Config` and makes `TestProps::load_from` read nicer. I've split these into separate commits, so I in theory could split these into separate PRs if they're controversial, but I think they're pretty straightforward. r? ``@Mark-Simulacrum``
2022-02-18Rollup merge of #93915 - Urgau:rfc-3013, r=petrochenkovMatthias Krüger-0/+102
Implement --check-cfg option (RFC 3013), take 2 This pull-request implement RFC 3013: Checking conditional compilation at compile time (https://github.com/rust-lang/rfcs/pull/3013) and is based on the previous attempt https://github.com/rust-lang/rust/pull/89346 by `@mwkmwkmwk` that was closed due to inactivity. I have address all the review comments from the previous attempt and added some more tests. cc https://github.com/rust-lang/rust/issues/82450 r? `@petrochenkov`
2022-02-18Rollup merge of #93892 - compiler-errors:issue-92917, r=jackh726,nikomatsakisMatthias Krüger-10/+37
Only mark projection as ambiguous if GAT substs are constrained A slightly more targeted version of #92917, where we only give up with ambiguity if we infer something about the GATs substs when probing for a projection candidate. fixes #93874 also note (but like the previous PR, does not fix) #91762 r? `@jackh726` cc `@nikomatsakis` who reviewed #92917
2022-02-18Rollup merge of #92806 - compiler-errors:better-impl-trait-deny, r=estebankMatthias Krüger-126/+126
Add more information to `impl Trait` error Fixes #92458 Let me know if I went overboard here, or if the suggestions could use some refinement. r? `@estebank` Feel free to reassign to someone else
2022-02-18Rollup merge of #91675 - ivanloz:memtagsan, r=nagisaMatthias Krüger-1/+1
Add MemTagSanitizer Support Add support for the LLVM [MemTagSanitizer](https://llvm.org/docs/MemTagSanitizer.html). On hardware which supports it (see caveats below), the MemTagSanitizer can catch bugs similar to AddressSanitizer and HardwareAddressSanitizer, but with lower overhead. On a tag mismatch, a SIGSEGV is signaled with code SEGV_MTESERR / SEGV_MTEAERR. # Usage `-Zsanitizer=memtag -C target-feature="+mte"` # Comments/Caveats * MemTagSanitizer is only supported on AArch64 targets with hardware support * Requires `-C target-feature="+mte"` * LLVM MemTagSanitizer currently only performs stack tagging. # TODO * Tests * Example
2022-02-18Rollup merge of #89892 - Nilstrieb:suggest-return-impl-trait, r=jackh726Matthias Krüger-0/+184
Suggest `impl Trait` return type when incorrectly using a generic return type Address #85991 When there is a type mismatch error and the return type is generic, and that generic parameter is not used in the function parameters, suggest replacing that generic with the `impl Trait` syntax. r? `@estebank`
2022-02-18Suggest `impl Trait` return typeNilstrieb-0/+184
Address #85991 Suggest the `impl Trait` return type syntax if the user tried to return a generic parameter and we get a type mismatch The suggestion is not emitted if the param appears in the function parameters, and only get the bounds that actually involve `T: ` directly It also checks whether the generic param is contained in any where bound (where it isn't the self type), and if one is found (like `Option<T>: Send`), it is not suggested. This also adds `TyS::contains`, which recursively vistits the type and looks if the other type is contained anywhere
2022-02-18Rollup merge of #93024 - compiler-errors:inline-mir-bad-bounds, r=estebankMatthias Krüger-0/+22
Do not ICE when inlining a function with un-satisfiable bounds Fixes #93008 This is kinda a hack... but it's the fix I thought had the least blast-radius. We use `normalize_param_env_or_error` to verify that the predicates in the param env are self-consistent, since with RevealAll, a bad predicate like `<&'static () as Clone>` will be evaluated with an empty ParamEnv (since it references no generics), and we'll raise an error for it.
2022-02-18Rollup merge of #92959 - asquared31415:test-non-fn-help, r=estebankMatthias Krüger-35/+265
Add more info and suggestions to use of #[test] on invalid items This pr changes the diagnostics for using `#[test]` on an item that can't be used as a test to explain that the attribute has no meaningful effect on non-functions and suggests the use of `#[cfg(test)]` for conditional compilation instead. Example change: ```rs #[test] mod test {} ``` previously output ``` error: only functions may be used as tests --> src/lib.rs:2:1 | 2 | mod test {} | ^^^^^^^^^^^ ``` now outputs ``` error: the `#[test]` attribute may only be used on a non-associated function --> $DIR/test-on-not-fn.rs:3:1 | LL | #[test] | ^^^^^^^ LL | mod test {} | ----------- expected a non-associated function, found a module | = note: the `#[test]` macro causes a a function to be run on a test and has no effect on non-functions help: replace with conditional compilation to make the item only exist when tests are being run | LL | #[cfg(test)] | ~~~~~~~~~~~~ ```
2022-02-18Rollup merge of #92683 - jackh726:issue-92033, r=estebankMatthias Krüger-25/+77
Suggest copying trait associated type bounds on lifetime error Closes #92033 Kind of the most simple suggestion to make - we don't try to be fancy. Turns out, it's still pretty useful (the couple existing tests that trigger this error end up fixed - for this error - upon applying the fix). r? ``@estebank`` cc ``@nikomatsakis``
2022-02-18Auto merge of #94088 - oli-obk:revert, r=jackh726bors-133/+190
Revert #91403 fixes #94004 r? `@pnkfelix` `@cjgillot`
2022-02-17skip pointing out ambuguous impls in alloc/std crates tooMichael Goulet-43/+17
2022-02-17fix impl trait message, bless testsMichael Goulet-252/+126
2022-02-17Add more information to `impl Trait` deny errorMichael Goulet-120/+246
2022-02-17Rollup merge of #94043 - DrMeepster:box_alloc_ice, r=oli-obkMatthias Krüger-0/+6
Fix ICE when using Box<T, A> with pointer sized A Fixes #78459 Note that using `Box<T, A>` with a more than pointer sized `A` or using a pointer sized `A` with a Box of a DST will produce a different ICE (#92054) which is not fixed by this PR.
2022-02-17Rollup merge of #93337 - Amanieu:asm_tracking, r=tmiaskoMatthias Krüger-4/+4
Update tracking issue numbers for inline assembly sub-features The main tracking issue for inline assembly is [closed](https://github.com/rust-lang/rust/issues/72016#issuecomment-1022332954), further tracking of the remaining sub-features has been moved to separate tracking issues.
2022-02-17Suggest copying trait associated type bounds on lifetime errorJack Huey-25/+77
2022-02-17Revert "Auto merge of #91403 - cjgillot:inherit-async, r=oli-obk"Oli Scherer-133/+190
This reverts commit 3cfa4def7c87d571bd46d92fed608edf8fad236e, reversing changes made to 5d8767cb229b097fedb1dd4bd9420d463c37774f.
2022-02-17Auto merge of #93577 - nikic:llvm-14, r=nagisabors-2/+2
Upgrade to LLVM 14 LLVM patch state: * [x] https://github.com/llvm/llvm-project/commit/a55727f334b39600bfc71144b11b42aae6b94e0b Backported. * [x] https://github.com/rust-lang/llvm-project/commit/c3c82dc12402dd41441180c0c6cf7aed7e330c53 Backported as https://github.com/llvm/llvm-project/commit/917c47b3bf0dfc45a2a5ba12c1397d647ecf4017. * [x] https://github.com/rust-lang/llvm-project/commit/6e8f9ab632d12271355d10d34c9835a7ba14e4b9 No plan to upstream. * [x] https://github.com/llvm/llvm-project/commit/319f4b2d52e31b000db75a0a2484b5f2ab90534a Backported. * [x] https://github.com/rust-lang/llvm-project/commit/8b2c25d321f877161f85218479e2d1317d770e18 No plan to upstream. * [x] https://github.com/rust-lang/llvm-project/commit/75fef2efd427362c8f16b2d09e6ebf44069e3919 No plan to upstream. * [ ] https://github.com/rust-lang/llvm-project/commit/adef757547de5a570d9f6a00d3e6ac16c666ab79 Upstreamed as https://github.com/llvm/llvm-project/commit/2d2ef384b2f6e723edb793d08f52e7f4dc94ba3a. Needs backport. * [x] https://github.com/rust-lang/llvm-project/commit/4b7c1b4910e9fa9e04f23f06be078e168ef4c0ee No plan to upstream. * [x] https://github.com/rust-lang/llvm-project/commit/3f5ab0c061adb723f25b94243828b6b5407720c8 No plan to upstream. * [x] https://github.com/rust-lang/llvm-project/commit/514d05500e0e15e358f05f5c4cec78a805858f8e No plan to upstream. * [ ] https://github.com/rust-lang/llvm-project/commit/54c586958564582b3341d1838a5de86541e5fecf Under review at https://reviews.llvm.org/D119695 and https://reviews.llvm.org/D119856. Release timeline: * LLVM 14.0.0 final planned for Mar 15. * Rust 1.60.0 planned for Apr 7. Compile-time: * https://perf.rust-lang.org/compare.html?start=250384edc5d78533e993f38c60d64e42b21684b2&end=b87df8d2c7c5d9ac448c585de10927ab2ee1b864 * A slight improvement on average, though no big changes either way. * There are some larger max-rss improvements. r? `@ghost`
2022-02-17Rollup merge of #94031 - ↵Matthias Krüger-27/+37
danielhenrymantilla:diagnostics/union-drop-suggest-copy-bound-alternative, r=davidtwco [diagnostics] Add mentions to `Copy` types being valid for `union` fields This came up from some user on Discord which was using a `T : PrimitiveInt` generic type, and they wanted to use in a `union`. Rather than adding a `Copy` bound, they started pondering about the `ManuallyDrop<T>` road, and how to correctly use `unsafe` to perform the drops. <img width="648" alt="Screen Shot 2022-02-15 at 22 28 34" src="https://user-images.githubusercontent.com/9920355/154152496-8f9be74b-ad59-4724-8f9e-48b446774e06.png"> - [Discord link](https://discord.com/channels/442252698964721669/443150878111694848/943092778534072320) So, it seemed like the error message for types with potential drop glue on `union` fields could be improved to also mention the `Copy` alternative, since in many cases where `union`s are concerned, people are dealing with PODs / `Copy` types anyways 🙂 ___ ``@rustbot`` modify labels: +A-diagnostics +D-terse
2022-02-17Rollup merge of #94030 - ChayimFriedman2:issue-94010, r=petrochenkovMatthias Krüger-42/+69
Correctly mark the span of captured arguments in `format_args!()` It should not include the braces, or misspelling suggestions will be wrong. Fixes #94010.
2022-02-17Rollup merge of #93996 - notriddle:notriddle/magically-becomes-a-function, ↵Matthias Krüger-0/+94
r=petrochenkov Do not suggest "is a function" for free variables Part of #82323
2022-02-17Rollup merge of #93981 - ChayimFriedman2:slice-pat-reference-option-result, ↵Matthias Krüger-1/+30
r=davidtwco Fix suggestion to slice if scurtinee is a reference to `Result` or `Option` Fixes https://github.com/rust-lang/rust/pull/91343#issuecomment-1037718339 and https://github.com/rust-lang/rust/pull/91343#discussion_r761466979.
2022-02-17Rollup merge of #93693 - rukai:91550, r=davidtwcoMatthias Krüger-12/+125
Suggest deriving required supertraits closes https://github.com/rust-lang/rust/issues/91550 I chose to just hardcode handling for PartialOrd and PartialEq because that should be robust enough and I dont know how to go about doing it generically r? rust-lang/diagnostics
2022-02-17Auto merge of #94040 - Mark-Simulacrum:destabilize-load-store, r=Amanieubors-0/+71
Destabilize cfg(target_has_atomic_load_store = ...) This was not intended to be stabilized yet. This keeps the cfg_target_has_atomic feature gate name since compiler-builtins otherwise depends on it and I'd rather not try to manage a bump across a crates.io published repository given the time-sensitivity here (we need to land this quickly to avoid a beta backport). Closes https://github.com/rust-lang/rust/issues/32976 r? `@Amanieu`
2022-02-16Consider mutations as borrows in drop trackingEric Holk-0/+114
This is needed to match MIR more conservative approximation of any borrowed value being live across a suspend point (See #94067). This change considers an expression such as `x.y = z` to be a borrow of `x` and therefore keeps `x` live across suspend points.
2022-02-16Update optimization remark test for LLVM 14Nikita Popov-2/+2
The format of the remark changed slightly.
2022-02-16Do not suggest "is a function" for free variablesMichael Howell-0/+94
Part of #82323
2022-02-16Destabilize cfg(target_has_atomic_load_store = ...)Mark Rousskov-0/+71
This was not intended to be stabilized yet.
2022-02-16MemTagSanitizer SupportIvan Lozano-1/+1
Adds support for the LLVM MemTagSanitizer.
2022-02-16allow special behavior when printing const inferlcnr-4/+4
2022-02-16Add mentions to `Copy` for `union` fieldsDaniel Henry-Mantilla-27/+37
2022-02-16Implement --check-cfg option (RFC 3013)Loïc BRANSTETT-0/+102
Co-authored-by: Urgau <lolo.branstett@numericable.fr> Co-authored-by: Marcelina Kościelnicka <mwk@0x04.net>
2022-02-16Correctly mark the span of captured arguments in `format_args!()`Chayim Refael Friedman-42/+69
It should only include the identifier, or misspelling suggestions will be wrong.
2022-02-15fix assumption that ScalarPair Box is always a fat pointerDrMeepster-0/+6
2022-02-16Make implementation genericLucas Kent-16/+16
2022-02-16Suggest deriving required supertraitsLucas Kent-0/+113
2022-02-16Fix inconsistent symbol mangling with -ZverboseTomasz Miąsko-8/+23
Always skip arguments that are the defaults of their respective parameters, to avoid generating inconsistent symbols for builds with `-Zverbose` flag and without it.
2022-02-15Auto merge of #93820 - compiler-errors:gat-wfcheck, r=jackh726bors-4/+34
Rework GAT `where` clause check rework the GAT where check to use a fixed-point algorithm, and check all GATs in a trait at once fixes #93278 r? `@jackh726` cc `@nikomatsakis`
2022-02-15Add removed comments back in self-outlives-lintMichael Goulet-15/+18
2022-02-15add test for issue-93278, blessMichael Goulet-17/+44
2022-02-15Rollup merge of #93999 - barzamin:suggest-raw-strings, r=jackh726Matthias Krüger-0/+29
suggest using raw strings when invalid escapes appear in literals i'd guess about 70% of "bad escape" cases occur when someone meant to use a raw string literal because they're passing it directly to `Regex::new()`. this emits an advisory (`Applicability::MaybeIncorrect`) `help:` suggestion to the user that they use an `r""` string, on top of the normal notes about looking at the string literal documentation/spec.
2022-02-15Auto merge of #93148 - nnethercote:Uniq, r=fee1-deadbors-4/+4
Overhaul interning. A number of types are interned and `eq` and `hash` are implemented on the pointer rather than the contents. But this is not well enforced within the type system like you might expect. This PR introduces a new type `Interned` which encapsulates this concept more rigorously, and uses it to convert a couple of the less common interned types. r? `@fee1-dead`
2022-02-15Auto merge of #93176 - danielhenrymantilla:stack-pinning-macro, r=m-ou-sebors-0/+115
Add a stack-`pin!`-ning macro to `core::pin`. - https://github.com/rust-lang/rust/issues/93178 `pin!` allows pinning a value to the stack. Thanks to being implemented in the stdlib, which gives access to `macro` macros, and to the private `.pointer` field of the `Pin` wrapper, [it was recently discovered](https://rust-lang.zulipchat.com/#narrow/stream/187312-wg-async-foundations/topic/pin!.20.E2.80.94.20the.20.22definitive.22.20edition.20.28a.20rhs-compatible.20pin-nin.2E.2E.2E/near/268731241) ([archive link](https://zulip-archive.rust-lang.org/stream/187312-wg-async-foundations/topic/A.20rhs-compatible.20pin-ning.20macro.html#268731241)), contrary to popular belief, that it is actually possible to implement and feature such a macro: ```rust let foo: Pin<&mut PhantomPinned> = pin!(PhantomPinned); stuff(foo); ``` or, directly: ```rust stuff(pin!(PhantomPinned)); ``` - For context, historically, this used to require one of the two following syntaxes: - ```rust let foo = PhantomPinned; pin!(foo); stuff(foo); ``` - ```rust pin! { let foo = PhantomPinned; } stuff(foo); ``` This macro thus allows, for instance, doing things like: ```diff fn block_on<T>(fut: impl Future<Output = T>) -> T { // Pin the future so it can be polled. - let mut fut = Box::pin(fut); + let mut fut = pin!(fut); // Create a new context to be passed to the future. let t = thread::current(); let waker = Arc::new(ThreadWaker(t)).into(); let mut cx = Context::from_waker(&waker); // Run the future to completion. loop { match fut.as_mut().poll(&mut cx) { Poll::Ready(res) => return res, Poll::Pending => thread::park(), } } } ``` - _c.f._, https://doc.rust-lang.org/1.58.1/alloc/task/trait.Wake.html And so on, and so forth. I don't think such an API can get better than that, barring full featured language support (`&pin` references or something), so I see no reason not to start experimenting with featuring this in the stdlib already 🙂 - cc `@rust-lang/wg-async-foundations` \[EDIT: this doesn't seem to have pinged anybody 😩, thanks `@yoshuawuyts` for the real ping\] r? `@joshtriplett` ___ # Docs preview https://user-images.githubusercontent.com/9920355/150605731-1f45c2eb-c9b0-4ce3-b17f-2784fb75786e.mp4 ___ # Implementation The implementation ends up being dead simple (so much it's embarrassing): ```rust pub macro pin($value:expr $(,)?) { Pin { pointer: &mut { $value } } } ``` _and voilà_! - The key for it working lies in [the rules governing the scope of anonymous temporaries](https://doc.rust-lang.org/1.58.1/reference/destructors.html#temporary-lifetime-extension). <details><summary>Comments and context</summary> This is `Pin::new_unchecked(&mut { $value })`, so, for starters, let's review such a hypothetical macro (that any user-code could define): ```rust macro_rules! pin {( $value:expr ) => ( match &mut { $value } { at_value => unsafe { // Do not wrap `$value` in an `unsafe` block. $crate::pin::Pin::<&mut _>::new_unchecked(at_value) }} )} ``` Safety: - `type P = &mut _`. There are thus no pathological `Deref{,Mut}` impls that would break `Pin`'s invariants. - `{ $value }` is braced, making it a _block expression_, thus **moving** the given `$value`, and making it _become an **anonymous** temporary_. By virtue of being anonynomous, it can no longer be accessed, thus preventing any attemps to `mem::replace` it or `mem::forget` it, _etc._ This gives us a `pin!` definition that is sound, and which works, but only in certain scenarios: - If the `pin!(value)` expression is _directly_ fed to a function call: `let poll = pin!(fut).poll(cx);` - If the `pin!(value)` expression is part of a scrutinee: ```rust match pin!(fut) { pinned_fut => { pinned_fut.as_mut().poll(...); pinned_fut.as_mut().poll(...); }} // <- `fut` is dropped here. ``` Alas, it doesn't work for the more straight-forward use-case: `let` bindings. ```rust let pinned_fut = pin!(fut); // <- temporary value is freed at the end of this statement pinned_fut.poll(...) // error[E0716]: temporary value dropped while borrowed // note: consider using a `let` binding to create a longer lived value ``` - Issues such as this one are the ones motivating https://github.com/rust-lang/rfcs/pull/66 This makes such a macro incredibly unergonomic in practice, and the reason most macros out there had to take the path of being a statement/binding macro (_e.g._, `pin!(future);`) instead of featuring the more intuitive ergonomics of an expression macro. Luckily, there is a way to avoid the problem. Indeed, the problem stems from the fact that a temporary is dropped at the end of its enclosing statement when it is part of the parameters given to function call, which has precisely been the case with our `Pin::new_unchecked()`! For instance, ```rust let p = Pin::new_unchecked(&mut <temporary>); ``` becomes: ```rust let p = { let mut anon = <temporary>; &mut anon }; ``` However, when using a literal braced struct to construct the value, references to temporaries can then be taken. This makes Rust change the lifespan of such temporaries so that they are, instead, dropped _at the end of the enscoping block_. For instance, ```rust let p = Pin { pointer: &mut <temporary> }; ``` becomes: ```rust let mut anon = <temporary>; let p = Pin { pointer: &mut anon }; ``` which is *exactly* what we want. Finally, we don't hit problems _w.r.t._ the privacy of the `pointer` field, or the unqualified `Pin` name, thanks to `decl_macro`s being _fully_ hygienic (`def_site` hygiene). </details> ___ # TODO - [x] Add compile-fail tests with attempts to break the `Pin` invariants thanks to the macro (_e.g._, try to access the private `.pointer` field, or see what happens if such a pin is used outside its enscoping scope (borrow error)); - [ ] Follow-up stuff: - [ ] Try to experiment with adding `pin!` to the prelude: this may require to be handled with some extra care, as it may lead to issues reminiscent of those of `assert_matches!`: https://github.com/rust-lang/rust/issues/82913 - [x] Create the tracking issue.