Skip to content

/rust

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

Un-regress conflicting destructors #28681

Merged
merged 4 commits into from Sep 28, 2015

Conversation

Reviewers
No reviews
Assignees

@eddyb eddyb

Labels
I-needs-decision T-compiler beta-accepted
Projects
None yet
Milestone
No milestone
7 participants
@arielb1 @eddyb @bors @brson @pnkfelix @nikomatsakis @alexcrichton
@arielb1
Copy link
Contributor

arielb1 commented Sep 26, 2015

Fixes #28568

r? @eddyb

Ariel Ben-Yehuda and others added some commits Sep 24, 2015

Ariel Ben-Yehuda @arielb1
deduplicate trait errors before they are displayed 
Because of type inference, duplicate obligations exist and cause duplicate
errors. To avoid this, only display the first error for each (predicate,span).

The inclusion of the span is somewhat bikesheddy, but *is* the more
conservative option (it does not remove some instability, as duplicate
obligations are ignored by `duplicate_set` under some inference conditions).

Fixes #28098
cc #21528 (is it a dupe?)
fe6ad09
Ariel Ben-Yehuda @arielb1
show each object-safety violation once 
different supertraits can suffer from the same object-safety violation,
leading to duplication in the error message. Avoid it.

Fixes #20692
Loading status checks…
346088b
@arielb1
remove the destructors table
f9b703e
@arielb1
don't crash when there are multiple conflicting implementations of Drop 
Fixes #28568
Loading status checks…
9a86713

@rust-highfive rust-highfive assigned eddyb Sep 26, 2015

@eddyb

This comment has been minimized.

Sign in to view
Copy link
Member

eddyb commented Sep 26, 2015

@bors r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Sep 26, 2015

📌 Commit 9a86713 has been approved by eddyb

Manishearth added a commit to Manishearth/rust that referenced this pull request Sep 27, 2015

@Manishearth
Rollup merge of rust-lang#28681 - arielb1:destructor-fixes, r=eddyb 
Fixes rust-lang#28568 

r? @eddyb
7e6e64b

@Manishearth Manishearth referenced this pull request Sep 27, 2015

Merged

Rollup of 5 pull requests #28689

bors added a commit that referenced this pull request Sep 27, 2015

@bors
Auto merge of #28689 - Manishearth:rollup, r=Manishearth 
- Successful merges: #28664, #28673, #28681, #28682, #28688
- Failed merges:
Loading status checks…
638b260

@alexcrichton alexcrichton merged commit 9a86713 into rust-lang:master Sep 28, 2015
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@brson brson added the beta-nominated label Oct 2, 2015

@brson

This comment has been minimized.

Sign in to view
Copy link
Contributor

brson commented Oct 2, 2015

The linked issue indicates this regression is on beta. Backport? cc @rust-lang/compiler

@brson brson added I-needs-decision T-compiler labels Oct 2, 2015

@pnkfelix

This comment has been minimized.

Sign in to view
Copy link
Member

pnkfelix commented Oct 16, 2015

@rust-lang/compiler I have no opinion about backporting this to beta; I certainly don't object, but I also don't think its the end of the world if an ICE remains.

(That's assuming one always hits an ICE ... if someone were to demonstrate an example where the program has two destructors and the compiler erroneously accepts it, then that would change my opinion to be strongly in favor of a backport.)
@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor

nikomatsakis commented Oct 16, 2015

I would not ordinarily be in favor of backporting, but if it's a regression I think we ought to.

@nikomatsakis nikomatsakis added the beta-accepted label Oct 16, 2015

@brson brson referenced this pull request Oct 16, 2015

Merged

Beta next #29112

@alexcrichton alexcrichton removed the beta-nominated label Oct 16, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.