Add `known-bug` tests for 11 unsound issues #110480

whtahy · 2023-04-18T04:39:12Z

Should tests for other issues be in separate PRs? Thanks.

Edit: Partially addresses #105107. This PR adds known-bug tests for 11 unsound issues:

rustbot · 2023-04-18T04:39:20Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jackh726 (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

jackh726

Thanks! Just a comment on the header comments.

For other issues, feel free to add them here, or you can open separate PRs.

jackh726 · 2023-04-18T05:18:27Z

tests/ui/implied-bounds/implied-bounds-on-nested-references-plus-variance.rs

+// submitted by @aturon: The combination of variance and implied bounds for
+// nested references opens a hole in the current type system
+// known-bug: #25860
+// check-pass
+// unsoundness issue: should-fail
+
+// mcve from @pnkfelix
+// https://github.com/rust-lang/rust/issues/25860#issue-82044592


Probably too much here. Here's what I would write:

// check-pass // known-bug #23860 // Should fail. The combination of variance and implied bounds for // nested references allows us to infer a longer lifetime than we can prove.

Aha, thanks! Updated.

whtahy · 2023-04-18T06:23:47Z

Hmm, I'm thinking I should open a new pull request, since it looks like issue # in commit msg is spamming the issues?

kadiwa4 · 2023-04-18T07:19:14Z

You can squash your commits and edit the message on the way

jackh726 · 2023-04-18T13:21:56Z

Yeah, are you able to squash the commits into just one? And then you can edit the message to not tag the issue (just make the commit message like "add known-bug test for issue 25860).

Then, we can tag the issue in the PR description.

Then, if you want to add more tests, basically repeat, with one test per commit? (But I'm happy to merge with one test and followup later with others if you're interested)

whtahy · 2023-04-18T22:05:02Z

You can squash your commits and edit the message on the way

Yes! This is great, thanks.

Yeah, are you able to squash the commits into just one? And then you can edit the message to not tag the issue (just make the commit message like "add known-bug test for issue 25860).

Then, if you want to add more tests, basically repeat, with one test per commit?

Sounds good, will do.

Then, we can tag the issue in the PR description.

Ah, so I should edit the top comment after all the tests are in?

I appreciate the help. Let me know if there's anything else.

jackh726 · 2023-04-20T23:45:40Z

Ah, so I should edit the top comment after all the tests are in?
I'm not sure if github will relink when editing the comment or not. So up to you. (I'm also about to review and can tag them if I r+)

I appreciate the help. Let me know if there's anything else.

Of course! Thanks for the contribution.

jackh726

Just some nits on one test then LGTM.

jackh726 · 2023-04-20T23:49:43Z

tests/ui/impl-trait/indirect-impl-for-trait-obj-coherence.rs

@@ -0,0 +1,28 @@
+// check-pass


This test isn't in the right place (impl-trait tests are for impl Trait syntax).

Maybe just under tests/ui/coherence

Thanks, moved to tests/ui/coherence as suggested.

jackh726 · 2023-04-20T23:49:58Z

tests/ui/impl-trait/indirect-impl-for-trait-obj-coherence.rs

+// Coherence = given a trait and some set of types for its type parameters,
+// there should be *exactly one* impl that applies.


This comment isn't needed

Alright, removed.

whtahy · 2023-04-21T03:08:52Z

Alright! Hopefully good to go.

updated and squashed the 1 test
added issues to PR description

jackh726 · 2023-04-22T01:10:26Z

tests/ui/traits/negative-impls/pin-unsound-issue-85099-derefmut.rs

@@ -0,0 +1,68 @@
+// check-pass


Agh just saw that this test is not in the right place. Let's just put it under tests/ui/typeck or tests/ui/issues. (tests/ui/deref would be nicer, but not worth creating a whole directory for only one test)

No worries, moved to tests/ui/typeck.

jackh726 · 2023-04-22T01:11:20Z

tests/ui/static/static-closures-with-nonstatic-return.rs

@@ -0,0 +1,15 @@
+// check-pass


Let's also move this to tests/ui/closures, static is for static items, I think.

Aha, moved to tests/ui/closures.

whtahy · 2023-04-22T18:08:21Z

Added 4 new tests and updated PR description. That's all the tests for this PR. Happy to make changes, move things around, etc.

jackh726 · 2023-04-23T16:51:26Z

Thanks! This is great.

@bors r+ rollup

bors · 2023-04-23T16:51:28Z

📌 Commit ebe61ce has been approved by jackh726

It is now in the queue for this repository.

…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? `@jackh726` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005

…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? ``@jackh726`` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005

jackh726 · 2023-04-24T03:19:04Z

note to self: need to add S-bug-has-test to the issues covered by this PR when this lands

…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? `@jackh726` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005

…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? ``@jackh726`` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005

…unsound-issues, r=jackh726 Add `known-bug` tests for 11 unsound issues r? `@jackh726` Should tests for other issues be in separate PRs? Thanks. Edit: Partially addresses rust-lang#105107. This PR adds `known-bug` tests for 11 unsound issues: - rust-lang#25860 - rust-lang#49206 - rust-lang#57893 - rust-lang#84366 - rust-lang#84533 - rust-lang#84591 - rust-lang#85099 - rust-lang#98117 - rust-lang#100041 - rust-lang#100051 - rust-lang#104005

Rollup of 10 pull requests Successful merges: - rust-lang#110480 (Add `known-bug` tests for 11 unsound issues) - rust-lang#110539 (Move around `{Idx, IndexVec, IndexSlice}` adjacent code) - rust-lang#110590 (Add some tests around (lack of) object safety of associated types and consts) - rust-lang#110602 (Ignore src/bootstrap formatting commit in .git-blame-ignore-revs) - rust-lang#110667 (pointer-auth-link-with-c: Fix cross compilation.) - rust-lang#110681 (drop few unused crates, gate libc under unix for rustc_codegen_ssa) - rust-lang#110685 (Some cleanups to DataflowConstProp) - rust-lang#110744 (bootstrap: update paths cargo-credential crate) - rust-lang#110750 (Add size asserts for MIR `SourceScopeData` & `VarDebugInfo`) - rust-lang#110760 (rustdoc: Add regression test for rust-lang#60522) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned jackh726 Apr 18, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 18, 2023

jackh726 requested changes Apr 18, 2023

View reviewed changes

whtahy force-pushed the 105107/known-bug-tests-for-unsound-issues branch from 66b3ae7 to a535aa8 Compare April 18, 2023 05:57

This comment has been minimized.

Sign in to view

whtahy force-pushed the 105107/known-bug-tests-for-unsound-issues branch from d8c832f to 7f7f00c Compare April 18, 2023 22:17

whtahy marked this pull request as draft April 18, 2023 22:29

whtahy force-pushed the 105107/known-bug-tests-for-unsound-issues branch from 7f7f00c to 7cd2696 Compare April 18, 2023 22:30

jackh726 reviewed Apr 20, 2023

View reviewed changes

whtahy force-pushed the 105107/known-bug-tests-for-unsound-issues branch 2 times, most recently from 9b33d6b to af3a6be Compare April 21, 2023 02:55

whtahy marked this pull request as ready for review April 21, 2023 03:02

jackh726 requested changes Apr 22, 2023

View reviewed changes

whtahy added 6 commits April 22, 2023 00:47

add known-bug test for unsound issue 25860

adb5ded

add known-bug test for unsound issue 49206

2fb2098

add known-bug test for unsound issue 57893

232d685

add known-bug test for unsound issue 84366

fbfb620

add known-bug test for unsound issue 84533

cac62ab

add known-bug test for unsound issue 84591

be68c69

whtahy force-pushed the 105107/known-bug-tests-for-unsound-issues branch from af3a6be to 5674295 Compare April 22, 2023 04:47

add known-bug test for unsound issue 85099

3c5de9a

whtahy force-pushed the 105107/known-bug-tests-for-unsound-issues branch from 5674295 to 3c5de9a Compare April 22, 2023 04:52

add known-bug test for unsound issue 104005

ebe61ce

whtahy marked this pull request as ready for review April 22, 2023 17:58

whtahy changed the title ~~#105107: add known-bug test for unsound issue #25860~~ Add known-bug tests for 11 unsound issues Apr 22, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 23, 2023

matthiaskrgr mentioned this pull request Apr 23, 2023

Rollup of 5 pull requests #110735

Closed

matthiaskrgr mentioned this pull request Apr 24, 2023

Rollup of 7 pull requests #110746

Closed

matthiaskrgr mentioned this pull request Apr 24, 2023

Rollup of 7 pull requests #110753

Closed

matthiaskrgr mentioned this pull request Apr 24, 2023

Rollup of 6 pull requests #110757

Closed

matthiaskrgr mentioned this pull request Apr 24, 2023

Rollup of 8 pull requests #110772

Closed

JohnTitor mentioned this pull request Apr 24, 2023

Rollup of 10 pull requests #110778

Merged

bors merged commit 42467d5 into rust-lang:master Apr 24, 2023

rustbot added this to the 1.71.0 milestone Apr 24, 2023

whtahy deleted the 105107/known-bug-tests-for-unsound-issues branch April 25, 2023 04:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `known-bug` tests for 11 unsound issues #110480

Add `known-bug` tests for 11 unsound issues #110480

whtahy commented Apr 18, 2023 •

edited

Loading

rustbot commented Apr 18, 2023

jackh726 left a comment

jackh726 Apr 18, 2023

whtahy Apr 18, 2023

whtahy commented Apr 18, 2023

This comment has been minimized.

kadiwa4 commented Apr 18, 2023

jackh726 commented Apr 18, 2023

whtahy commented Apr 18, 2023

jackh726 commented Apr 20, 2023

jackh726 left a comment

jackh726 Apr 20, 2023

whtahy Apr 21, 2023

jackh726 Apr 20, 2023

whtahy Apr 21, 2023

whtahy commented Apr 21, 2023

jackh726 Apr 22, 2023

whtahy Apr 22, 2023

jackh726 Apr 22, 2023

whtahy Apr 22, 2023

whtahy commented Apr 22, 2023

jackh726 commented Apr 23, 2023

bors commented Apr 23, 2023

jackh726 commented Apr 24, 2023

		// Coherence = given a trait and some set of types for its type parameters,
		// there should be exactly one impl that applies.

Add known-bug tests for 11 unsound issues #110480

Add known-bug tests for 11 unsound issues #110480

Conversation

whtahy commented Apr 18, 2023 • edited Loading

rustbot commented Apr 18, 2023

jackh726 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whtahy commented Apr 18, 2023

This comment has been minimized.

kadiwa4 commented Apr 18, 2023

jackh726 commented Apr 18, 2023

whtahy commented Apr 18, 2023

jackh726 commented Apr 20, 2023

jackh726 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whtahy commented Apr 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whtahy commented Apr 22, 2023

jackh726 commented Apr 23, 2023

bors commented Apr 23, 2023

jackh726 commented Apr 24, 2023

Add `known-bug` tests for 11 unsound issues #110480

Add `known-bug` tests for 11 unsound issues #110480

whtahy commented Apr 18, 2023 •

edited

Loading