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

fix: Fix layout for hir_ty::Ty and friends #14802

Merged
merged 1 commit into from
May 18, 2023
Merged

Conversation

HKalbasi
Copy link
Member

No description provided.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2023
Comment on lines +1975 to +1987
struct St2;

impl Tr for St2 {
type Ty = i64;
}

struct Goal(St<St2>);

let x = Goal(St(5));
Copy link
Member Author

Choose a reason for hiding this comment

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

It is strange for me that we support this. We don't pass a trait env with block anywhere in the layout code, but we can find the layout of Goal here. Anyway, I added this test to keep this behavior.

Copy link
Member

Choose a reason for hiding this comment

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

The layout code receives the substitutions though, which would come from the type inference code which does account for blocks.

Comment on lines +1972 to +1978
// if we move `St2` out of body, the test will fail, as we don't see the impl anymore. That
// case will probably be rejected by rustc in some later edition, but we should support this
// case.
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean only moving out the St2 struct definition (leaving the impl for St2 in the block)? That's surprising that that does not work. I'm pretty sure it should ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I mean keeping the impl inside of the block. You can reproduce that by hovering on goal. I guess the problem is that we don't pass the block of the goal, only the crate, so it can't normalize the associated type. But with this logic, this one shouldn't work either...

@HKalbasi
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented May 15, 2023

📌 Commit 1e2dcfb has been approved by HKalbasi

It is now in the queue for this repository.

@HKalbasi
Copy link
Member Author

@bors r-

@HKalbasi
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented May 15, 2023

📌 Commit 1e2dcfb has been approved by HKalbasi

It is now in the queue for this repository.

@lnicola
Copy link
Member

lnicola commented May 16, 2023

@bors retry

@lnicola
Copy link
Member

lnicola commented May 16, 2023

@bors r-

@lnicola
Copy link
Member

lnicola commented May 16, 2023

@bors r=HKalbasi

@bors
Copy link
Contributor

bors commented May 16, 2023

📌 Commit 1e2dcfb has been approved by [HKalbasi](https://github.com/HKalbasi)

It is now in the queue for this repository.

@HKalbasi
Copy link
Member Author

@bors r-

@HKalbasi
Copy link
Member Author

@bors r=SomeString

@bors
Copy link
Contributor

bors commented May 16, 2023

📌 Commit 1e2dcfb has been approved by SomeString

It is now in the queue for this repository.

@HKalbasi
Copy link
Member Author

@bors r-
@bors r+

@bors
Copy link
Contributor

bors commented May 16, 2023

📌 Commit 1e2dcfb has been approved by HKalbasi

It is now in the queue for this repository.

@lnicola
Copy link
Member

lnicola commented May 16, 2023

It's broken in most places, rust-lang/homu#191.

@bors
Copy link
Contributor

bors commented May 18, 2023

⌛ Testing commit 1e2dcfb with merge 8080008...

bors added a commit that referenced this pull request May 18, 2023
Fix layout for `hir_ty::Ty` and friends
@bors
Copy link
Contributor

bors commented May 18, 2023

💔 Test failed - checks-actions

@bors
Copy link
Contributor

bors commented May 18, 2023

⌛ Testing commit 1e2dcfb with merge c425bf8...

bors added a commit that referenced this pull request May 18, 2023
Fix layout for `hir_ty::Ty` and friends
@Veykril
Copy link
Member

Veykril commented May 18, 2023

@bors r-

@bors
Copy link
Contributor

bors commented May 18, 2023

💔 Test failed - checks-actions

@HKalbasi
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented May 18, 2023

📌 Commit 261047d has been approved by HKalbasi

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented May 18, 2023

⌛ Testing commit 261047d with merge 1c0235e...

@bors
Copy link
Contributor

bors commented May 18, 2023

☀️ Test successful - checks-actions
Approved by: HKalbasi
Pushing 1c0235e to master...

@bors bors merged commit 1c0235e into rust-lang:master May 18, 2023
@bors bors mentioned this pull request May 18, 2023
@lnicola lnicola changed the title Fix layout for hir_ty::Ty and friends fix: Fix layout for hir_ty::Ty and friends May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants