Fix debug assertion in typeck #72714

JohnTitor · 2020-05-28T23:30:19Z

rust-highfive · 2020-05-28T23:30:22Z

(rust_highfive has picked a reviewer for you, use r? to override)

varkor · 2020-05-30T13:46:31Z

I'd like to see a comment explaining why this extra condition is necessary. Also, I'm not sure whether ignoring it completely is the right way to go.

r? @eddyb, who originally added this predicate.

JohnTitor · 2020-07-02T01:53:48Z

My initial thought was that we could apply the same way as #72019 but yeah, now, I'm not sure if it's always true. @eddyb could you confirm this?

eddyb · 2020-07-15T00:44:06Z

src/librustc_typeck/collect.rs

-                if bound_pred.bounds.is_empty() {
+                if bound_pred.bounds.is_empty() && !ty.has_escaping_bound_vars() {


Huh, this was previously changed from a WF predicate, to an outlives one?

I suspect instead of changing the condition here (which would hide e.g. where for<'a> Foo<'a>:, from WF), you just need to change the ty::Binder::dummy to ty::Binder::bind.

Thanks! That should work fine.

eddyb · 2020-07-15T00:44:50Z

src/test/ui/traits/issue-72410.rs

+    fn map()
+    where
+        Self: Sized,
+        for<'a> &'a mut [u8]: ;


We should have a test for a type that is not WF, which this PR would allow compiling, but we'd never want to.

Should we separate that test in an another file? And I'm not sure what kind of test case is appropriate.

Perhaps something like

trait Foo<'a> { } fn map() where for<'a> &'a mut [dyn Foo<'a>]

which I think ought to fail because dyn Foo<'a>: Sized doesn't hold

you could add it in the same file I suppose

Thanks Niko! Added it.

eddyb · 2020-07-15T00:47:10Z

My initial thought was that we could apply the same way as #72019 but yeah, now, I'm not sure if it's always true. @eddyb could you confirm this?

@JohnTitor that change was in suggestion logic, whereas here it's for enforcing WF, and ICEs are preferred to unsoundness.

I left a comment with my suggestion (dummy -> bind) for making this work, I hope that works (since I didn't actually test it).

r? @nikomatsakis or @matthewjasper

nikomatsakis · 2020-07-17T19:56:03Z

This looks right to me but I second the request for a test that reports an (expected) error.

nikomatsakis · 2020-07-20T13:54:43Z

@bors r+

bors · 2020-07-20T13:54:44Z

📌 Commit a11024f has been approved by nikomatsakis

bors · 2020-07-20T18:16:30Z

⌛ Testing commit a11024f with merge e6f0d96fae0a2b8cd40b6df8ce54c69ddc488603...

bors · 2020-07-20T18:54:21Z

💔 Test failed - checks-actions

JohnTitor · 2020-07-20T18:57:02Z

Failed job (https://github.com/rust-lang-ci/rust/runs/891114144) says:

To https://github.com/pietroalbini/rust-toolstate
 ! [remote rejected] master -> master (permission denied)
error: failed to push some refs to 'https://github.com/pietroalbini/rust-toolstate'
Sleeping for 3 seconds before retrying push
From https://github.com/pietroalbini/rust-toolstate
 * branch            master     -> FETCH_HEAD
HEAD is now at 3158691 (windows CI update)
thread 'main' panicked at 'Failed to update toolstate repository with new data', src/bootstrap/toolstate.rs:447:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test check-tools
Build completed unsuccessfully in 0:00:33

Seems spurious, @bors retry

@ghost

…arth Rollup of 13 pull requests Successful merges: - rust-lang#72714 (Fix debug assertion in typeck) - rust-lang#73197 (Impl Default for ranges) - rust-lang#73323 (wf: check foreign fn decls for well-formedness) - rust-lang#74051 (disallow non-static lifetimes in const generics) - rust-lang#74376 (test caching opt_const_param_of on disc) - rust-lang#74501 (Ayu theme: Use different background color for Run button) - rust-lang#74505 (Fix search input focus in ayu theme) - rust-lang#74522 (Update sanitizer docs) - rust-lang#74546 (Fix duplicate maybe_uninit_extra attribute) - rust-lang#74552 (Stabilize TAU constant.) - rust-lang#74555 (Improve "important traits" popup display on mobile) - rust-lang#74557 (Fix an ICE on an invalid `binding @ ...` in a tuple struct pattern) - rust-lang#74561 (update backtrace-rs) Failed merges: r? @ghost

Original message: Rollup merge of rust-lang#72714 - JohnTitor:debug-assert, r=nikomatsakis Fix debug assertion in typeck Fixes rust-lang#72410

rust-highfive assigned varkor May 28, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 28, 2020

rust-highfive assigned eddyb and unassigned varkor May 30, 2020

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 6, 2020

Elinvynia added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 25, 2020

JohnTitor force-pushed the debug-assert branch from 20493a9 to 42ab2d2 Compare June 30, 2020 23:37

JohnTitor added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 30, 2020

eddyb reviewed Jul 15, 2020

View reviewed changes

rust-highfive assigned nikomatsakis and unassigned eddyb Jul 15, 2020

JohnTitor force-pushed the debug-assert branch from 42ab2d2 to 54a7264 Compare July 15, 2020 03:08

nikomatsakis approved these changes Jul 17, 2020

View reviewed changes

Fix debug assertion in typeck

a11024f

JohnTitor force-pushed the debug-assert branch from 54a7264 to a11024f Compare July 18, 2020 03:21

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 Jul 20, 2020

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

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 Jul 20, 2020

Manishearth mentioned this pull request Jul 20, 2020

Rollup of 13 pull requests #74569

Merged

bors merged commit 105cd49 into rust-lang:master Jul 21, 2020

JohnTitor deleted the debug-assert branch July 21, 2020 06:32

rust-timer added a commit to rust-timer/rust that referenced this pull request Jul 21, 2020

rust-timer simulated merge of 105cd49

6354c29

Original message: Rollup merge of rust-lang#72714 - JohnTitor:debug-assert, r=nikomatsakis Fix debug assertion in typeck Fixes rust-lang#72410

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix debug assertion in typeck #72714

Fix debug assertion in typeck #72714

JohnTitor commented May 28, 2020

rust-highfive commented May 28, 2020

varkor commented May 30, 2020 •

edited

Loading

JohnTitor commented Jul 2, 2020

eddyb Jul 15, 2020

JohnTitor Jul 15, 2020

eddyb Jul 15, 2020

JohnTitor Jul 16, 2020

nikomatsakis Jul 17, 2020

nikomatsakis Jul 17, 2020

JohnTitor Jul 18, 2020

eddyb commented Jul 15, 2020

nikomatsakis commented Jul 17, 2020

nikomatsakis commented Jul 20, 2020

bors commented Jul 20, 2020

bors commented Jul 20, 2020

bors commented Jul 20, 2020

JohnTitor commented Jul 20, 2020

		if bound_pred.bounds.is_empty() {
		if bound_pred.bounds.is_empty() && !ty.has_escaping_bound_vars() {

Fix debug assertion in typeck #72714

Fix debug assertion in typeck #72714

Conversation

JohnTitor commented May 28, 2020

rust-highfive commented May 28, 2020

varkor commented May 30, 2020 • edited Loading

JohnTitor commented Jul 2, 2020

eddyb Jul 15, 2020

Choose a reason for hiding this comment

JohnTitor Jul 15, 2020

Choose a reason for hiding this comment

eddyb Jul 15, 2020

Choose a reason for hiding this comment

JohnTitor Jul 16, 2020

Choose a reason for hiding this comment

nikomatsakis Jul 17, 2020

Choose a reason for hiding this comment

nikomatsakis Jul 17, 2020

Choose a reason for hiding this comment

JohnTitor Jul 18, 2020

Choose a reason for hiding this comment

eddyb commented Jul 15, 2020

nikomatsakis commented Jul 17, 2020

nikomatsakis commented Jul 20, 2020

bors commented Jul 20, 2020

bors commented Jul 20, 2020

bors commented Jul 20, 2020

JohnTitor commented Jul 20, 2020

varkor commented May 30, 2020 •

edited

Loading