Implement Return Type Notation (RTN)'s path form in where clauses #129629

compiler-errors · 2024-08-26T21:18:14Z

Implement return type notation (RTN) in path position for where clauses. We already had RTN in associated type position (e.g.), but per the RFC:

As a standalone type, RTN can only be used as the Self type of a where-clause [...]

Specifically, in order to enable code like:

trait Foo {
    fn bar() -> impl Sized;
}

fn is_send(_: impl Send) {}

fn test<T>()
where
    T: Foo,
    T::bar(..): Send,
{
    is_send(T::bar());
}

In the resolver, when we see a TyKind::Path whose final segment is GenericArgs::ParenthesizedElided (i.e. (..)), resolve that path in the value namespace, since we're looking for a method.
When lowering where clauses in HIR lowering, we first try to intercept an RTN self type via lower_ty_maybe_return_type_notation. If we find an RTN type, we lower it manually in a way that respects its higher-ranked-ness (see below) and resolves to the corresponding RPITIT. Anywhere else, we'll emit the same "return type notation not allowed in this position yet" error we do when writing RTN in every other position.
In resolve_bound_vars, we add some special treatment for RTN types in where clauses. Specifically, we need to add new lifetime variables to our binders for the early- and late-bound vars we encounter on the method. This implements the higher-ranked desugaring laid out in the RFC.

This PR also adds a bunch of tests, mostly negative ones (testing error messages).

In a follow-up PR, I'm going to mark RTN as no longer incomplete, since this PR basically finishes the impl surface that we should initially stabilize, and the RFC was accepted.

cc RFC 3654 and #109417

rustbot · 2024-08-26T21:18:22Z

r? @jackh726

rustbot has assigned @jackh726.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-08-26T21:18:24Z

HIR ty lowering was modified

cc @fmease

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

compiler/rustc_hir_analysis/src/errors.rs

compiler/rustc_ast_lowering/src/lib.rs

compiler-errors · 2024-09-17T18:22:06Z

Hi @jackh726. It's been 3 weeks since this was opened, so I'm pinging just to ask whether you're willing and free to review this PR, or if I should try to find someone else to review it.

jackh726 · 2024-09-17T18:28:24Z

Sorry! Yes, I'll take a look sometime this week.

jackh726

I think I probably want to do a second pass over this, but I do think some more tests here are needed.

tests/ui/suggestions/let-binding-init-expr-as-ty.stderr

jackh726 · 2024-09-21T01:43:32Z

compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs

+    // and a bound that looks like:
+    //    `for<'a, 'b> <T as Trait<'a>>::x(): Other<'b>`
+    // this is going to expand to something like:
+    //    `for<'a, 'b, 'r, T> <T as Trait<'a>>::x::<'r, T>::{opaque#0}: Other<'b>`.


Are there actually tests for this? I see one test to deny type parameters, but not for lifetimes.

This comment is a bit out of date. We don't support type params anyways, but tests/ui/associated-type-bounds/return-type-notation/higher-ranked-bound-works.rs exercises when we have bound vars in both the trait ref, the method, and the goal.

compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs

jackh726 · 2024-09-21T01:50:29Z

compiler/rustc_resolve/src/late.rs

+                // If we have a path that ends with `(..)`, then it must be
+                // return type notation. Resolve that path in the *value*
+                // namespace.


This is fairly fragile, right? If, for example, the user write method(), we won't resolve things correctly.

At the very least, I would expect a test for this.

What specifically would you like tested here? When there's a conflicting definition in the type namespace?

I've added tests/ui/associated-type-bounds/return-type-notation/namespace-conflict.rs to ensure that we're not resolving to something in the type namespace (namely, a GAT).

IDK about method() by itself, but that needs to be resolved in the type namespace for bare Fn() trait objects. If you meant T::method() or <T as Trait>::method(): the diagnostics for the former (which we lower as a type-dependent path in HIR) are nice, but the latter (which we lower in resolve) are not. I can fix that as a follow-up, I guess :)

Can you add a test for what diagnostic we emit if the user writes method(): Send?

Should be covered by tests/ui/associated-type-bounds/return-type-notation/bad-inputs-and-output.rs and tests/ui/associated-type-bounds/return-type-notation/not-a-method.rs. The diagnostics could definitely use work on the resolver side, but I'll defer that to a follow-up.

jackh726 · 2024-09-21T02:04:45Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs

+        // Next, we need to check that the return-type notation is being used on
+        // an RPITIT (return-position impl trait in trait) or AFIT (async fn in trait).


This is...not a restriction I expect given the RFC:

The primary use case is to add bounds such as Send to the futures returned by async fns in traits and -> impl Future functions, but they work for any trait function defined with return-position impl trait (e.g., where T: Factory<widgets(..): DoubleEndedIterator> would also be valid).

At the very least, this should have a test.

This is a restriction specifically called out in the RFC: https://rust-lang.github.io/rfcs/3654-return-type-notation.html#rtn-only-applies-to-afit-and-rpitit-methods

I've extended tests/ui/associated-type-bounds/return-type-notation/non-rpitit.rs to exercise this codepath, which was already exercised for the associated-type-bound flavor of RTN.

This is a restriction specifically called out in the RFC: https://rust-lang.github.io/rfcs/3654-return-type-notation.html#rtn-only-applies-to-afit-and-rpitit-methods

Hmm, that quoted text is from the RFC, the summary section. That should probably be amended.

jackh726

Just the one comment remaining about adding a test for the mistyped method(): Send diagnostic, then r=me.

compiler-errors · 2024-09-21T14:12:38Z

tests/ui/associated-type-bounds/return-type-notation/not-a-method.stderr

@@ -0,0 +1,49 @@
+error[E0575]: expected function, found function `function`


Namely, this is a consequence of E0575 calling associated functions "functions".

As a follow-up, I will do like #130414 and probably split these resolver errors into their own error codes, so they can have tailored explanations.

compiler-errors · 2024-09-21T15:56:34Z

@bors r=jackh726

bors · 2024-09-21T15:56:37Z

📌 Commit 1de894f has been approved by jackh726

It is now in the queue for this repository.

…kh726 Implement Return Type Notation (RTN)'s path form in where clauses Implement return type notation (RTN) in path position for where clauses. We already had RTN in associated type position ([e.g.](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=627a4fb8e2cb334863fbd08ed3722c09)), but per [the RFC](https://rust-lang.github.io/rfcs/3654-return-type-notation.html#where-rtn-can-be-used-for-now): > As a standalone type, RTN can only be used as the Self type of a where-clause [...] Specifically, in order to enable code like: ```rust trait Foo { fn bar() -> impl Sized; } fn is_send(_: impl Send) {} fn test<T>() where T: Foo, T::bar(..): Send, { is_send(T::bar()); } ``` * In the resolver, when we see a `TyKind::Path` whose final segment is `GenericArgs::ParenthesizedElided` (i.e. `(..)`), resolve that path in the *value* namespace, since we're looking for a method. * When lowering where clauses in HIR lowering, we first try to intercept an RTN self type via `lower_ty_maybe_return_type_notation`. If we find an RTN type, we lower it manually in a way that respects its higher-ranked-ness (see below) and resolves to the corresponding RPITIT. Anywhere else, we'll emit the same "return type notation not allowed in this position yet" error we do when writing RTN in every other position. * In `resolve_bound_vars`, we add some special treatment for RTN types in where clauses. Specifically, we need to add new lifetime variables to our binders for the early- and late-bound vars we encounter on the method. This implements the higher-ranked desugaring [laid out in the RFC](https://rust-lang.github.io/rfcs/3654-return-type-notation.html#converting-to-higher-ranked-trait-bounds). This PR also adds a bunch of tests, mostly negative ones (testing error messages). In a follow-up PR, I'm going to mark RTN as no longer incomplete, since this PR basically finishes the impl surface that we should initially stabilize, and the RFC was accepted. cc [RFC 3654](rust-lang/rfcs#3654) and rust-lang#109417

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127766 (add `extern "C-cmse-nonsecure-entry" fn` ) - rust-lang#129629 (Implement Return Type Notation (RTN)'s path form in where clauses) - rust-lang#130246 (rustc_expand: remember module `#[path]`s during expansion) - rust-lang#130408 (Avoid re-validating UTF-8 in `FromUtf8Error::into_utf8_lossy`) - rust-lang#130651 (Add --enable-profiler to armhf dist) - rust-lang#130653 (ABI compatibility: mention Result guarantee) - rust-lang#130665 (Prevent Deduplication of `LongRunningWarn`) - rust-lang#130666 (Assert that `explicit_super_predicates_of` and `explicit_item_super_predicates` truly only contains bounds for the type itself) - rust-lang#130667 (compiler: Accept "improper" ctypes in extern "rust-cold" fn) r? `@ghost` `@rustbot` modify labels: rollup

…mpiler-errors Rollup of 8 pull requests Successful merges: - rust-lang#127766 (add `extern "C-cmse-nonsecure-entry" fn` ) - rust-lang#129629 (Implement Return Type Notation (RTN)'s path form in where clauses) - rust-lang#130408 (Avoid re-validating UTF-8 in `FromUtf8Error::into_utf8_lossy`) - rust-lang#130651 (Add --enable-profiler to armhf dist) - rust-lang#130653 (ABI compatibility: mention Result guarantee) - rust-lang#130666 (Assert that `explicit_super_predicates_of` and `explicit_item_super_predicates` truly only contains bounds for the type itself) - rust-lang#130667 (compiler: Accept "improper" ctypes in extern "rust-cold" fn) - rust-lang#130673 (Parser: recover from `:::` to `::`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#129629 - compiler-errors:rtn-in-path, r=jackh726 Implement Return Type Notation (RTN)'s path form in where clauses Implement return type notation (RTN) in path position for where clauses. We already had RTN in associated type position ([e.g.](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=627a4fb8e2cb334863fbd08ed3722c09)), but per [the RFC](https://rust-lang.github.io/rfcs/3654-return-type-notation.html#where-rtn-can-be-used-for-now): > As a standalone type, RTN can only be used as the Self type of a where-clause [...] Specifically, in order to enable code like: ```rust trait Foo { fn bar() -> impl Sized; } fn is_send(_: impl Send) {} fn test<T>() where T: Foo, T::bar(..): Send, { is_send(T::bar()); } ``` * In the resolver, when we see a `TyKind::Path` whose final segment is `GenericArgs::ParenthesizedElided` (i.e. `(..)`), resolve that path in the *value* namespace, since we're looking for a method. * When lowering where clauses in HIR lowering, we first try to intercept an RTN self type via `lower_ty_maybe_return_type_notation`. If we find an RTN type, we lower it manually in a way that respects its higher-ranked-ness (see below) and resolves to the corresponding RPITIT. Anywhere else, we'll emit the same "return type notation not allowed in this position yet" error we do when writing RTN in every other position. * In `resolve_bound_vars`, we add some special treatment for RTN types in where clauses. Specifically, we need to add new lifetime variables to our binders for the early- and late-bound vars we encounter on the method. This implements the higher-ranked desugaring [laid out in the RFC](https://rust-lang.github.io/rfcs/3654-return-type-notation.html#converting-to-higher-ranked-trait-bounds). This PR also adds a bunch of tests, mostly negative ones (testing error messages). In a follow-up PR, I'm going to mark RTN as no longer incomplete, since this PR basically finishes the impl surface that we should initially stabilize, and the RFC was accepted. cc [RFC 3654](rust-lang/rfcs#3654) and rust-lang#109417

…=pnkfelix Robustify and genericize return-type-notation resolution in `resolve_bound_vars` rust-lang#129629 implemented return-type-notation (RTN) in its path form, like `where T::method(..): Bound`. As part of lowering, we must record the late-bound vars for the where clause introduced by the method (namely, its early- and late-bound lifetime arguments, since `where T::method(..)` turns into a higher-ranked where clause over all of the lifetimes according to [RFC 3654](https://rust-lang.github.io/rfcs/3654-return-type-notation.html#converting-to-higher-ranked-trait-bounds)). However, this logic was only looking at the where clauses of the parent item that the `T::method(..)` bound was written on, and not any parent items. This PR generalizes that logic to look at the parent item (i.e. the outer impl or trait) instead and fixes a (debug only) assertion as an effect. This logic is also more general and likely easier to adapt to more interesting (though likely very far off) cases like non-lifetime binder `for<T: Trait> T::method(..): Send` bounds. Tracking: - rust-lang#109417

rustbot assigned jackh726 Aug 26, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 26, 2024

compiler-errors force-pushed the rtn-in-path branch from 2f67719 to 7da572e Compare August 26, 2024 21:18

compiler-errors mentioned this pull request Aug 26, 2024

Tracking Issue for return type notation #109417

Open

7 tasks

fmease reviewed Aug 26, 2024

View reviewed changes

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs Show resolved Hide resolved

fmease reviewed Aug 26, 2024

View reviewed changes

compiler/rustc_hir_analysis/src/errors.rs Show resolved Hide resolved

compiler/rustc_ast_lowering/src/lib.rs Outdated Show resolved Hide resolved

compiler-errors force-pushed the rtn-in-path branch from 7da572e to 5911914 Compare August 26, 2024 22:11

compiler-errors force-pushed the rtn-in-path branch from 5911914 to b24a7a9 Compare September 17, 2024 00:18

compiler-errors mentioned this pull request Sep 19, 2024

Bring the Async Rust experience closer to parity with sync Rust rust-lang/rust-project-goals#105

Open

40 tasks

carloskiki mentioned this pull request Sep 21, 2024

tower-sessions-core crate rework maxcountryman/tower-sessions#218

Closed

jackh726 reviewed Sep 21, 2024

View reviewed changes

compiler-errors added 6 commits September 20, 2024 22:18

Resolve RTN for TyKind::Path ending in (..)

6d788a1

Conditionally allow lowering RTN (..) in paths

19881b5

Implement RTN in resolve_bound_vars and HIR ty lowering

51b51bb

Flesh out some TODOs

7c8e281

Add missing diagnostics and flesh out tests

174c3f9

Resolve self type alias in impl for RTN

af24d0b

compiler-errors force-pushed the rtn-in-path branch from b24a7a9 to e2fc2a1 Compare September 21, 2024 03:00

jackh726 approved these changes Sep 21, 2024

View reviewed changes

More tests and tweak comments

1de894f

compiler-errors force-pushed the rtn-in-path branch from e2fc2a1 to 1de894f Compare September 21, 2024 14:10

compiler-errors commented Sep 21, 2024

View reviewed changes

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 21, 2024

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 21, 2024

matthiaskrgr mentioned this pull request Sep 21, 2024

Rollup of 9 pull requests #130671

Closed

compiler-errors mentioned this pull request Sep 21, 2024

Rollup of 8 pull requests #130674

Merged

bors merged commit d72d44d into rust-lang:master Sep 22, 2024
6 checks passed

rustbot added this to the 1.83.0 milestone Sep 22, 2024

compiler-errors mentioned this pull request Oct 23, 2024

Robustify and genericize return-type-notation resolution in resolve_bound_vars #132047

Open

tikue mentioned this pull request Oct 27, 2024

Implement an easier way to get Send Serve and Stub google/tarpc#480

Open

matthiaskrgr mentioned this pull request Nov 2, 2024

ICE when using RTN in the where clause of a trait #132159

Open

traviscross added the F-return_type_notation `#[feature(return_type_notation)]` label Nov 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Return Type Notation (RTN)'s path form in where clauses #129629

Implement Return Type Notation (RTN)'s path form in where clauses #129629

compiler-errors commented Aug 26, 2024 •

edited

Loading

rustbot commented Aug 26, 2024

rustbot commented Aug 26, 2024

compiler-errors commented Sep 17, 2024

jackh726 commented Sep 17, 2024

jackh726 left a comment

jackh726 Sep 21, 2024

compiler-errors Sep 21, 2024

jackh726 Sep 21, 2024

jackh726 Sep 21, 2024

compiler-errors Sep 21, 2024

compiler-errors Sep 21, 2024

jackh726 Sep 21, 2024

compiler-errors Sep 21, 2024

jackh726 Sep 21, 2024

compiler-errors Sep 21, 2024

compiler-errors Sep 21, 2024

jackh726 Sep 21, 2024

jackh726 left a comment

compiler-errors Sep 21, 2024

compiler-errors commented Sep 21, 2024

bors commented Sep 21, 2024

		// Next, we need to check that the return-type notation is being used on
		// an RPITIT (return-position impl trait in trait) or AFIT (async fn in trait).

		@@ -0,0 +1,49 @@
		error[E0575]: expected function, found function `function`

Implement Return Type Notation (RTN)'s path form in where clauses #129629

Implement Return Type Notation (RTN)'s path form in where clauses #129629

Conversation

compiler-errors commented Aug 26, 2024 • edited Loading

rustbot commented Aug 26, 2024

rustbot commented Aug 26, 2024

compiler-errors commented Sep 17, 2024

jackh726 commented Sep 17, 2024

jackh726 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jackh726 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors commented Sep 21, 2024

bors commented Sep 21, 2024

compiler-errors commented Aug 26, 2024 •

edited

Loading