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

regression: the size for values of type T cannot be known #125195

Closed
BoxyUwU opened this issue May 17, 2024 · 5 comments
Closed

regression: the size for values of type T cannot be known #125195

BoxyUwU opened this issue May 17, 2024 · 5 comments
Labels
regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@BoxyUwU
Copy link
Member

BoxyUwU commented May 17, 2024

[INFO] [stdout] error[E0277]: the size for values of type `T` cannot be known at compilation time
[INFO] [stdout]   --> src/group_map.rs:52:11
[INFO] [stdout]    |
[INFO] [stdout] 51 | pub(crate) fn right_with_right_id<T: PartialEq + ?Sized>(id: ID) -> impl Fn(&RightItem<T>) -> bool {
[INFO] [stdout]    |                                   - this type parameter needs to be `Sized`
[INFO] [stdout] 52 |     move |x| x.id == id
[INFO] [stdout]    |           ^ doesn't have a size known at compile-time
[INFO] [stdout]    |

probably #123531, cc @compiler-errors should be intended breakage

@BoxyUwU BoxyUwU added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels May 17, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 17, 2024
@BoxyUwU BoxyUwU added this to the 1.79.0 milestone May 17, 2024
@compiler-errors
Copy link
Member

Surprisingly more breakage than #123531, perhaps we should be doing craterbot-test instead of cratebot-check for types PRs. I don't think we should unland the PR tho.

@lqd
Copy link
Member

lqd commented May 17, 2024

perhaps we should be doing craterbot-test instead of cratebot-check

You mean, because you explicitly want codegen to happen in case there are errors late (or in doctests maybe)? Or because these cases were missing from #123531's results? It seems at least the first error was present (but invisible) in the crater run.

I wonder if cycle_map-0.2.0 may have denied warnings or something. In #123531's results, it failed to build both before and after, but failed with a new E0277. No status change, so it wasn't flagged as a regression:

{       
            "name": "cycle_map-0.2.0",
            "url": "https://crates.io/crates/cycle_map/0.2.0",
            "krate": {
                "Registry": {
                    "name": "cycle_map",
                    "version": "0.2.0"
                }
            },  
            "status": "",
            "res": "build-fail",
            "runs": [
                {
                    "res": "build-fail:compiler-error(unused_imports)",
                    "log": "master%2330840c53f414b753cb08f14620906572f88dda4b/reg/cycle_map-0.2.0"
                },
                {
                    "res": "build-fail:compiler-error(E0277, unused_imports)",
                    "log": "try%2318fb860b8a8e7ca508746882328061d0fe352543/reg/cycle_map-0.2.0"
                }
            ]
        },  

Beta crater runs cap lints while most crater runs don't, maybe doing that would slightly widen the reach in t-types runs.

(I gotta run but I'll check the other crates later today.)

@lqd
Copy link
Member

lqd commented May 17, 2024

  • sharded-0.2.1: known and errs opened a PR to fix it
  • Zersya.invoice-billing-server: marked build-fail for unknown reason, both before and after; but was actually failing because of proc-macro2-1.0.46, before reaching the PR's change. I think it may be because of the nightly feature detection? I'm imagining this: if the PR run was seen as a nightly, proc-macro2 enabled the proc_macro_span_shrink nightly feature flag and that caused the failure; while this didn't happen for the beta run, proc-macro2 successfully built, and we reached the PR's change.
  • alvarotolentino.docmanager-rust: marked build-fail for unknown reason, both before and after; it didn't actually build on nightly when Enforce closure args + return type are WF #123531 landed in the first place (a few "implementation is not general enough" errors in sqlx models). These errors bisect to instantiate higher ranked goals outside of candidate selection #119820 instead, and that repo was indeed in that PR's crater regressions.
  • pykulytsky.kvs: I believe this is nightly feature detection again, with ahash-0.7.7 enabling feature(stdsimd), causing the build to fail in the PR crater run.
  • thalber.bronze_loop: same as the above, with ahash-0.7.6.
  • scherben-map-0.1.10: marked as build fail for unknown reason, but the failure is actually in a dependency: sharded-0.1.0, a different rev of the first known item above.

@apiraino
Copy link
Contributor

ok this will be considered as acceptable breaking change and thus will be in the relnotes (comment)

@rustbot label -I-prioritize -needs-triage

@rustbot rustbot removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 28, 2024
@Mark-Simulacrum Mark-Simulacrum added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Oct 11, 2024
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 11, 2024
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 12, 2024
@apiraino
Copy link
Contributor

Closing since regression is mentioned in the release notes

@apiraino apiraino closed this as not planned Won't fix, can't repro, duplicate, stale Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants