-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add better tests for hidden lifetimes in impl trait #60756
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea to add a test! But since this test is kind of testing for a tricky scenario, it'd be good to explain the scenario in more detail, so when we trip over it, we can understand what happened. =)
@@ -0,0 +1,53 @@ | |||
// Test to show actual unsound behaviour that occurs when the hidden lifetime | |||
// check is removed from impl trait types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you cc the relevant issues/PRs here in the test, to help us track down more info later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or else expand this comment a bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or both? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like:
Test to show what would happen if we were not careful and permitted lifetimes to escape through an impl trait. In this case, we are returning a &'a mut &'b T
, but the 'b
does not appear in the impl trait type. We can use this to engineer an unsoundness. (XXX perhaps a more detailed explanation)
05ebc79
to
32c790c
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
32c790c
to
53e0474
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@bors r+ |
📌 Commit 53e0474 has been approved by |
@bors rollup |
…s, r=nikomatsakis Add better tests for hidden lifetimes in impl trait cc rust-lang#60670
…s, r=nikomatsakis Add better tests for hidden lifetimes in impl trait cc rust-lang#60670
…s, r=nikomatsakis Add better tests for hidden lifetimes in impl trait cc rust-lang#60670
Rollup of 9 pull requests Successful merges: - #60742 (Allow const parameters in array sizes to be unified) - #60756 (Add better tests for hidden lifetimes in impl trait) - #60928 (Changes the type `mir::Mir` into `mir::Body`) - #61024 (tests: Centralize proc macros commonly used for testing) - #61157 (BufReader: In Seek impl, remove extra discard_buffer call) - #61195 (Special-case `.llvm` in mangler) - #61202 (Print PermissionExt::mode() in octal in Documentation Examples) - #61259 (Mailmap fixes) - #61273 (mention that MaybeUninit is a bit like Option) Failed merges: r? @ghost
cc #60670