-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Further tighten up relaxed bounds #147734
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
base: master
Are you sure you want to change the base?
Conversation
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Further tighten up relaxed bounds
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (9ed9a73): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -3.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 474.854s -> 475.36s (0.11%) |
2453034
to
5fab794
Compare
@bors rollup- |
5fab794
to
c67d187
Compare
79993ba
to
86cd4b0
Compare
… was enabled The internal feature `more_maybe_bounds` doesn't influence sized elaboration in HIR ty lowering and therefore doesn't get to dictate where `?Sized` is allowed.
86cd4b0
to
65814c8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Completely" unrelated to feature sized_hierarchy
. Well, it must've once ICE'd during the development of sized_hierarchy
and the PR that introduced it did affect stable behavior.
In any case, since ?Sized
is now illegal inside trait alias bounds, this is now covered by test tests/ui/traits/alias/relaxed-bounds.rs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, we did not have any UI test for trait Empty =;
prior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superseded by tests/ui/traits/alias/effectively-empty-trait-object-type.rs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved into tests/ui/unsized/relaxed-bounds-invalid-places.rs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superseded by tests/ui/traits/alias/relaxed-bounds.rs
.
//~| ERROR bound modifier `?` can only be applied to `Sized` | ||
//~| ERROR bound modifier `?` can only be applied to `Sized` | ||
//~| ERROR bound modifier `?` can only be applied to `Sized` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate diagnostics (under -Zdeduplicate-diagnostics=no
) because we lower the supertraits multiple times during HIR ty lowering likely under different predicate filters (HIR ty lowering needs to do this sort of thing in a bunch of situations to avoid query cycles).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This test is just a mixed bag, always has been, and that's fine, the feature is internal and still evolving)
trait_ref.path.segments.split_last().unwrap().1.iter(), | ||
GenericsArgsErrExtend::None, | ||
); | ||
let [leading_segments @ .., segment] = trait_ref.path.segments else { bug!() }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive-by refactoring
This comment was marked as outdated.
This comment was marked as outdated.
r? compiler |
Follow-up to #142693, #135331 and #135841.
Fixes #143122.
?Trait
in the bounds of trait aliases.Just like
trait Trait {}
doesn't meantrait Trait: Sized {}
and we therefore rejecttrait Trait: ?Sized {}
,trait Trait =;
(sic!) doesn't meantrait Trait = Sized;
(never did!) and as logical consequencetrait Trait = ?Sized;
is meaningless and should be forbidden.?Sized
in more places (e.g., supertrait bounds, trait object types) if featuremore_maybe_bounds
is enabled.That internal feature is only meant to allow the user to define & use new default traits (that have fewer rules to follow for now to ease experimentation).
Trait
in?Trait
is a default trait.Previously, we would only perform this check in selected places which was very brittle and led to bugs slipping through.