Skip to content
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

universes refactor 3 #55305

Merged
merged 11 commits into from
Nov 2, 2018
Merged

universes refactor 3 #55305

merged 11 commits into from
Nov 2, 2018

Conversation

nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Oct 24, 2018

Some more refactorings from my universe branch. These are getting a bit more "invasive" -- they start to plumb the universe information through the canonicalization process. As of yet though I don't believe this branch changes our behavior in any notable way, though I'm marking the branch as WIP to give myself a chance to verify this.

r? @scalexm

@rust-highfive

This comment has been minimized.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 24, 2018
@nikomatsakis
Copy link
Contributor Author

r? @scalexm

@rust-highfive rust-highfive assigned scalexm and unassigned cramertj Oct 24, 2018
@rust-highfive

This comment has been minimized.

Region,
Region(ty::UniverseIndex),

/// Region variable `'?R`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you wanted the exact same comment as the one above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@nikomatsakis nikomatsakis changed the title [WIP] Universes refactor 3 universes refactor 3 Oct 24, 2018
@nikomatsakis
Copy link
Contributor Author

I'm removing the WIP tag. =) Though I would still like to spend a bit of time looking carefully over the code. But I guess the test suite passing is a good sign I've not changed much behavior here ;)

@nikomatsakis
Copy link
Contributor Author

There might be room though for an assert or two -- e.g., in the canonicalization code -- checking that we don't have any universes in places we don't expect them.

@scalexm
Copy link
Member

scalexm commented Oct 25, 2018

Nothing strikes me regarding changes of behavior.

r=me if you ever want to add those asserts

@nikomatsakis
Copy link
Contributor Author

@bors r=scalexm

@bors
Copy link
Contributor

bors commented Oct 26, 2018

📌 Commit bcc2a7ffdb96b57b24638f76fee4033ebed65c8b has been approved by scalexm

@bors 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 Oct 26, 2018
@bors
Copy link
Contributor

bors commented Oct 27, 2018

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

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 27, 2018
But.. we don't really use it for anything right now.
The idea here is that an incoming query may refer to some universes,
and they query response may contain fresh universes that go beyond
those. When we instantiate the query response in the caller's scope,
therefore, we map those new universes into fresh universes for the
caller.
In particular, we don't want to preserve the universes for the `'_`
variables that appear in there. And we don't expect to find any
placeholders, which justifies this as harmless.

(In particular, if you have a query like `Foo(!1, !2, ?3)`, then you
care about the universe of `?3`, since it may control whether `?3 =
!1` and `?3 = !2` is a valid answer. But without any placeholders, we
don't really care: any placeholders that would appear in the output
must therefore come from some fresh universe anyway.)
Required for test expect-fn-supply-fn.rs to pass; otherwise we have
unconstrained inference variables that get inferred to `'empty`.
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 29, 2018
This was referenced Oct 29, 2018
@bors
Copy link
Contributor

bors commented Oct 31, 2018

⌛ Testing commit 36516e8ea82205a4f3b121518714353b15bb3c6f with merge f9c51413cb5661f89a6d12a3d05efdbb9d82c3bc...

@pietroalbini
Copy link
Member

pietroalbini commented Oct 31, 2018

Bors was testing two PRs at once.

@bors retry

Account for incompatible universes and higher-ranked subtyping.
@nikomatsakis
Copy link
Contributor Author

@bors r=scalexm

@bors
Copy link
Contributor

bors commented Oct 31, 2018

📌 Commit c244fd7 has been approved by scalexm

@nikomatsakis
Copy link
Contributor Author

@bors p=1

@scalexm requested that I give this p=1, as they have a number of things blocked on it

@bors
Copy link
Contributor

bors commented Nov 1, 2018

⌛ Testing commit c244fd7 with merge 29f782cbf921d0478683873670eb68e1739fe96c...

@bors
Copy link
Contributor

bors commented Nov 1, 2018

💔 Test failed - status-appveyor

@bors 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 Nov 1, 2018
@scalexm
Copy link
Member

scalexm commented Nov 1, 2018

yet another timeout, not the same job as the first one, and the build times seem to be varying randomly, so I believe this is not due to a perf problem?

@pietroalbini
Copy link
Member

@bors retry

@bors 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 Nov 1, 2018
@bors
Copy link
Contributor

bors commented Nov 2, 2018

⌛ Testing commit c244fd7 with merge 5eda136...

bors added a commit that referenced this pull request Nov 2, 2018
universes refactor 3

Some more refactorings from my universe branch. These are getting a bit more "invasive" -- they start to plumb the universe information through the canonicalization process. As of yet though I don't **believe** this branch changes our behavior in any notable way, though I'm marking the branch as `WIP` to give myself a chance to verify this.

r? @scalexm
@bors
Copy link
Contributor

bors commented Nov 2, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: scalexm
Pushing 5eda136 to master...

@bors bors merged commit c244fd7 into rust-lang:master Nov 2, 2018
@notriddle
Copy link
Contributor

For the benefit of the viewing audience:

This is a part of #48049 (remove the leak check in favor of "universes")

@tmandry tmandry added the WG-traits Working group: Traits, https://internals.rust-lang.org/t/announcing-traits-working-group/6804 label May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. WG-traits Working group: Traits, https://internals.rust-lang.org/t/announcing-traits-working-group/6804
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants