Skip to content

Conversation

@arielb1
Copy link
Contributor

@arielb1 arielb1 commented Jun 15, 2015

This adds transaction support to fulfill. I can't use it in #26282
because that would require nested transactions.

This doesn't seem to cause compilation time to regress.

Fixes #24819.

This adds transaction support to fulfill. I can't use it in rust-lang#26282
because that would require nested transactions.

This doesn't seem to cause compilation time to regress.

Fixes rust-lang#24819.
@rust-highfive
Copy link
Contributor

r? @pcwalton

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

@arielb1
Copy link
Contributor Author

arielb1 commented Jun 15, 2015

r? @nikomatsakis

@bors
Copy link
Collaborator

bors commented Jun 18, 2015

☔ The latest upstream changes (presumably #26326) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis
Copy link
Contributor

So, now that the fulfillment context has moved into the inference context, I'd like to integrate it fully into the transactional mechanism there. Sounds like you have a similar ambition. There are other changes I'd like to make -- for example, to track more fully the "parentage" of implications so that we can handle the global caching in a better way, and give better error reports -- and I have the feeling a complete rewrite is in order.

I am torn between saying let's just do the "Right Thing" and landing this PR in the interim. I don't like adding another variant on commit and so forth, but closing an ICE is always nice. So I guess I'm mostly happy to r+ but I'd like to move sooner rather than later on a more full rewrite. (Happy to talk about what that entails on IRC.)

cc @jroesch

@Gankra
Copy link
Contributor

Gankra commented Jul 27, 2015

Any progress on the decision?

@steveklabnik
Copy link
Contributor

Triage ping. Looks like this isn't going anywhere, should it be closed? Or did some sort of decision happen?

@arielb1
Copy link
Contributor Author

arielb1 commented Sep 27, 2015

@steveklabnik

@jroesch is working on a replacement for this patch.

@nikomatsakis
Copy link
Contributor

going to close in favor of @jroesch's anticipated PR, thanks @arielb1

@jroesch
Copy link
Contributor

jroesch commented Sep 28, 2015

@nikomatsakis sounds good, should be soon finally settled in (and have importantly have internet).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants