default auto traits: use default supertraits instead of Self: Trait bounds on associated items
#145879
Conversation
rustbot
added
S-waiting-on-review
|
HIR ty lowering was modified cc @fmease |
| @@ -21,6 +21,20 @@ fn bar<T: ?Sized + ?Trait2 + ?Trait1 + ?Trait4>(_: &T) {} | |||
|
|
|||
| // FIXME: `?Trait1` should be rejected, `Trait1` isn't marked `#[lang = "default_traitN"]`. | |||
| fn baz<T>() where T: Iterator<Item: ?Trait1> {} | |||
| //~^ ERROR this relaxed bound is not permitted here | |||
There was a problem hiding this comment.
Moreover this test is quite a mixed bag, it's unclear what the test intentions are. In #142693 I almost deleted it for that reason but I kept it to track bugs pertaining to more_maybe_bounds instead (as can be seen by the top-level FIXME).
I don't think we should test "this relaxed bound is not permitted here" in this file, I'd advise you to create a new one whose name is based on tests/ui/unsized/relaxed-bounds-invalid-places.rs (obv not inside unsized/; maybe have both test files in trait-bounds/ actually).
There was a problem hiding this comment.
From my understanding there are 2 checks for relaxed bounds consistency:
- Syntactic check during AST->HIR lowering (
validate_relaxed_bound) to ensure that?Traitbounds are in a right place. - Semantic check during HIR->ty lowering (
check_and_report_invalid_relaxed_bounds) to ensure that?modifier is applied to "right" traits. And this check has to be done regardless of the place and results of the first check.
This PR fixes the first check but not the second and therefore FIXME comments in this file are still relevant.
So, your suggestion is to keep these checks in separate files for readability reasons, right? In my opinion it isn't very useful because the invocation of second check implies that the first check has already been invoked. That is, the test that covers the second check also covers the first one.
Bryanskiy
force-pushed
the
supertraits-2
branch
from
55e0947 to
b8175b6
Compare
This comment has been minimized.
This comment has been minimized.
|
|
||
| /// Lazily sets `experimental_default_bounds` to true on trait super bounds. | ||
| /// See `requires_default_supertraits` for more information. | ||
| /// Sets `experimental_default_bounds` to true on trait super bounds. |
There was a problem hiding this comment.
and what does "Sets experimental_default_bounds to true on trait super bounds." mean?
from what i can see the function is actually adding a trait bound, not setting anything to true
Bryanskiy
force-pushed
the
supertraits-2
branch
from
b8175b6 to
3ab7b39
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Rollup of 8 pull requests Successful merges: - #145327 (std: make address resolution weirdness local to SGX) - #145879 (default auto traits: use default supertraits instead of `Self: Trait` bounds on associated items) - #146123 (Suggest examples of format specifiers in error messages) - #146311 (Minor symbol comment fixes.) - #146322 (Make Barrier RefUnwindSafe again) - #146327 (Add tests for deref on pin) - #146340 (Strip frontmatter in fewer places) - #146342 (Improve C-variadic error messages: part 2) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #145879 - Bryanskiy:supertraits-2, r=lcnr default auto traits: use default supertraits instead of `Self: Trait` bounds on associated items First commit: the motivation has been discussed [here](#144679). Second commit: the only new places where new implicit `DefaultAutoTrait` bounds are generated are supertraits and trait object so `?Trait` syntax should be extended to these places only. r? `@lcnr`
First commit: the motivation has been discussed here.
Second commit: the only new places where new implicit
DefaultAutoTraitbounds are generated are supertraits and trait object so?Traitsyntax should be extended to these places only.r? @lcnr