Skip to content

Remove deferred sized checks (make them eager) #100652

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Aug 17, 2022

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Aug 16, 2022

Improves diagnostics spans... this doesn't seem to be the case anymore:

// Some additional `Sized` obligations badly affect type inference.
// These obligations are added in a later stage of typeck.
pub(super) deferred_sized_obligations:
        RefCell<Vec<(Ty<'tcx>, Span, traits::ObligationCauseCode<'tcx>)>>,

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 16, 2022
@rust-highfive
Copy link
Contributor

r? @TaKO8Ki

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 16, 2022
= help: unsized fn params are gated as an unstable feature
help: function arguments must have a statically known size, borrowed types always have a known size
|
LL | foo(&*x);
Copy link
Member Author

Choose a reason for hiding this comment

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

This can be followed up later

span,
traits::SizedArgumentType(None),
traits::SizedArgumentType(arg_span),
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a somewhat unrelated fix, but it improves spans as well I think.

@TaKO8Ki
Copy link
Member

TaKO8Ki commented Aug 17, 2022

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 17, 2022

📌 Commit 33212bf has been approved by TaKO8Ki

It is now in the queue for this repository.

@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 Aug 17, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 17, 2022
…cks, r=TaKO8Ki

Remove deferred sized checks (make them eager)

Improves diagnostics spans... this doesn't seem to be the case anymore:

```rust
// Some additional `Sized` obligations badly affect type inference.
// These obligations are added in a later stage of typeck.
pub(super) deferred_sized_obligations:
        RefCell<Vec<(Ty<'tcx>, Span, traits::ObligationCauseCode<'tcx>)>>,
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 17, 2022
…iaskrgr

Rollup of 15 pull requests

Successful merges:

 - rust-lang#99474 (Rustdoc json tests: New `@hasexact` test command)
 - rust-lang#99972 (interpret: only consider 1-ZST when searching for receiver)
 - rust-lang#100018 (Clean up `LitKind`)
 - rust-lang#100379 (triagebot: add translation-related mention groups)
 - rust-lang#100389 (Do not report cycle error when inferring return type for suggestion)
 - rust-lang#100489 (`is_knowable` use `Result` instead of `Option`)
 - rust-lang#100532 (unwind: don't build dependency when building for Miri)
 - rust-lang#100608 (needless separation of impl blocks)
 - rust-lang#100621 (Pass +atomics-32 feature for {arm,thumb}v4t-none-eabi)
 - rust-lang#100646 (Migrate emoji identifier diagnostics to `SessionDiagnostic` in rustc_interface)
 - rust-lang#100652 (Remove deferred sized checks (make them eager))
 - rust-lang#100655 (Update books)
 - rust-lang#100656 (Update cargo)
 - rust-lang#100660 (Fixed a few documentation errors)
 - rust-lang#100661 (Fixed a few documentation errors)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6eed54a into rust-lang:master Aug 17, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 17, 2022
@pnkfelix
Copy link
Member

Hey @compiler-errors , it seems like this PR might have caused some performance regressions, based on this comment from the rollup PR:

#100677 (comment)

The main benefit of this PR is improvements to the diagnostics, right? I'm trying to figure out whether the tradeoff works out here.

@pnkfelix
Copy link
Member

@rustbot label: +perf-regression

@rustbot rustbot added the perf-regression Performance regression. label Aug 24, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 3, 2022
…ed-sized-checks, r=pnkfelix

Revert "Remove deferred sized checks"

cc: rust-lang#100652 (comment)

I'm okay with reverting this for now, and I will look into the diagnostic regressions.

This reverts commit 33212bf.

r? `@pnkfelix`

----

EDIT: This _also_ fixes rust-lang#101066, a regression in method selection logic/coercion(?) due to the early registering of a `Sized` bound.
@compiler-errors compiler-errors deleted the no-defer-sized-checks branch August 11, 2023 20:10
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
…checks, r=pnkfelix

Revert "Remove deferred sized checks"

cc: rust-lang/rust#100652 (comment)

I'm okay with reverting this for now, and I will look into the diagnostic regressions.

This reverts commit 33212bf7f527798a8cfa2bbb38781742f4ca718a.

r? `@pnkfelix`

----

EDIT: This _also_ fixes #101066, a regression in method selection logic/coercion(?) due to the early registering of a `Sized` bound.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
…checks, r=pnkfelix

Revert "Remove deferred sized checks"

cc: rust-lang/rust#100652 (comment)

I'm okay with reverting this for now, and I will look into the diagnostic regressions.

This reverts commit 33212bf7f527798a8cfa2bbb38781742f4ca718a.

r? `@pnkfelix`

----

EDIT: This _also_ fixes #101066, a regression in method selection logic/coercion(?) due to the early registering of a `Sized` bound.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
perf-regression Performance regression. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants