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

Tracking Issue for "More Qualified Paths" #86935

Open
1 of 3 tasks
rylev opened this issue Jul 7, 2021 · 20 comments
Open
1 of 3 tasks

Tracking Issue for "More Qualified Paths" #86935

rylev opened this issue Jul 7, 2021 · 20 comments
Labels
A-associated-items Area: Associated items (types, constants & functions) A-parser Area: The parsing of Rust source code to an AST C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-more_qualified_paths `#![feature(more_qualified_paths)]` S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@rylev
Copy link
Member

rylev commented Jul 7, 2021

This is a tracking issue for the "more qualified paths" feature. This feature originally grew out of a bug report #79658 where structs with qualified paths could not be constructed or used in patterns.

For instance:

fn main() {
    let <Foo as MyTrait>::Assoc { a} = <Foo as MyTrait>::Assoc { a: 2 };
    // The above would previously not parse.
    assert!(a == 2);
}

struct StructStruct {
    a: i8,
}

trait MyTrait {
    type Assoc;
}

struct Foo;

impl MyTrait for Foo {
    type Assoc = StructStruct;
}

See the implementation PR #80080 for more detailed discussion.

The feature gate for the issue is #![feature(more_qualified_paths)].

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

Implementation history

#80080

@rylev rylev added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 7, 2021
@jonas-schievink jonas-schievink added A-associated-items Area: Associated items (types, constants & functions) A-parser Area: The parsing of Rust source code to an AST T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 7, 2021
m-ou-se added a commit to m-ou-se/rust that referenced this issue Jul 9, 2021
…, r=nagisa

Change linked tracking issue for more_qualified_paths

This updates the linked tracking issue for the `more_qualified_paths` feature from the implementation PR rust-lang#80080 to an actual tracking issue rust-lang#86935.
@Aaron1011
Copy link
Member

cc @petrochenkov: It looks like this might interact with my higher-ranked closure RFC: rust-lang/rfcs#3216 (comment)

Since only lifetimes can be specified within a for<> binder (which I doubt will ever change, since lifetimes are the only generic 'kind' that we can erase), I think we can still disambiguate between the two cases by checking for ' token immediately following the for < - a qualified path cannot start with a ', and a for<> binder can only start with a '.

Does that sound correct to you?

@comex
Copy link
Contributor

comex commented Mar 7, 2022

Since only lifetimes can be specified within a for<> binder (which I doubt will ever change, since lifetimes are the only generic 'kind' that we can erase)

Personally I'm really hoping to someday see closures with generic type parameters in Rust, similar to what exists in C++. There's no need to erase the type parameter; you "just" desugar to a generic Fn* impl, so, e.g.,

for<T> |x: T| { ... }

would be like

struct MyClosure;
impl<T> FnOnce<(T,)> for MyClosure {
    type Output = ();
    ...
}

Though, this is of limited use unless you also have higher-ranked trait bounds, like

fn takes_generic_closure<F>(f: F)
    where F: for<T> FnOnce(T)
{ ... }

But that too is something I would like to see someday.

@petrochenkov
Copy link
Contributor

@Aaron1011
This ambiguity already exists in the language in a few places, like impl <T> :: A {} and some others, and we have some disambiguation rules for this case.
I think we need to reuse the same rules for the for<...> closure case too, without introducing more special cases.

@rodrimati1992
Copy link
Contributor

rodrimati1992 commented Jun 4, 2022

Lack of stable support for this can be worked around with a simple type alias
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=92460c3c1d0931dc9fac47d8e9877a51

use std::ops::Range;

type Type<T> = T;

fn main() {
    let Type::<<Vec<Range<u8>> as IntoIterator>::Item> {start, end} =
        Type::<<Vec<Range<u8>> as IntoIterator>::Item> {start: 3, end: 5};
    
    assert_eq!(start, 3);
    assert_eq!(end, 5);
}

@jhpratt
Copy link
Member

jhpratt commented Jul 25, 2022

Where is this actually wanted? It seems extremely convoluted. Surely if you know the names of the fields, you know the name of the type? I was amazed to see this is already possible via the trivial workaround, and think this should absolutely not be encouraged.

@Aaron1011
Copy link
Member

The main use-case would be macros (for example, if you want to assert a type built from a user-provided type). However, I don't know if there are currently any macros in the wild that would benefit from this.

@tyranron
Copy link

The situation where we name a concrete type via an associative type is quite regular in macros, yes.

However, I don't know if there are currently any macros in the wild that would benefit from this.

At least, #[cucumber::when] does.

@joshtriplett joshtriplett added the S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR label Jul 27, 2022
@joshtriplett
Copy link
Member

This seems like it may be ready to stabilize. Could we get a stabilization report and documentation PR?

@jhpratt
Copy link
Member

jhpratt commented Jul 27, 2022

The situation where we name a concrete type via an associative type is quite regular in macros, yes.

Why is the proc macro unable to know the name of the concrete type?

@TehPers
Copy link

TehPers commented Jan 13, 2023

I just came across an issue working on a proc macro as well where this would have been useful. The proc macro knows (well, assumes) that the type implements trait Foo and needs to construct an instance of its associated type Options using fields provided in the macro:

let constructed = construct!(Bar, a = 12, b = 14);

// expansion:
let constructed = <Bar as Foo>::construct(
    <Bar as Foo>::Options { a:  12, b: 14 }
);

Naturally this is a simplified version of the proc macro I was working on. I know a macro exactly like this one would likely not be very useful.

@silvanshade
Copy link

I have a use case for this in a real world macro scenario for generating complex bridge code for Objective-C.

I have a macro that looks like this:

#[objc(error_enum,
    mod = ns_url_error,
    domain = unsafe { NSURLErrorDomain },
    user_info = [
        { key = NSURLErrorFailingURLErrorKey, type = Id<NSURL> },
        { key = NSURLErrorFailingURLStringErrorKey, type = Id<NSString> },
        { key = NSURLErrorBackgroundTaskCancelledReasonKey, type = ns_url_error_reasons::BackgroundTaskCancelledReason },
        { key = NSURLErrorNetworkUnavailableReasonKey, type = ns_url_error_reasons::NetworkUnavailableReason },
    ]
)]
pub enum NSURLError {
    // several variants
}

This generates a bunch of code that constructs an error interface similar to the Swift analogue for URLError. In particular, I generate a refinement of the userInfo dictionary for the associated NSError, which is a struct called UserInfo, which gets placed within a module generated alongside the rest of the code (here I explicitly name the module ns_url_error).

Later, I want the user to be able to pattern match on that UserInfo struct like this:

let <NSURLError as objc2::ErrorEnum>::UserInfo {
    failing_url,
    failing_url_string,
    background_task_cancelled_reason,
    network_unavailable_reason,
} = error.user_info();

Since there is a different UserInfo struct generated for each error type also generated this way, allowing this qualified syntax provides a uniform way for the user to refer to the specific UserInfo type without needing to know the name of the helper module.

@WaffleLapkin
Copy link
Member

I want to highlight that for some reason this feature does not work with unit and tuple structs, i.e. <Unit as Trait>::Assoc and <Tuple as Trait>::Assoc(a) won't be parsed (play).

This seems like a bug/missed thing, but at the same time we have a test for it. I couldn't find any reason why this restriction exists, am I missing something?

@petrochenkov
Copy link
Contributor

@WaffleLapkin
Well, <Tuple as Trait>::Assoc is a type, not a function or a value in general.
So it cannot be used in call expressions or path expressions, similarly to any other type.

@Robbepop
Copy link
Contributor

Robbepop commented Mar 26, 2023

Today I found a very confusing bug in the Rust compiler associated to this issue:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1a6362262d04ed118d2d36d381cb69d1

error: ambiguous associated item
  --> src/main.rs:26:32
   |
26 |             Self::Ref { a } => Self::Ref::Ref { a },
   |                                ^^^^^^^^^ help: use fully-qualified syntax: `<Test as EnumRef>::Ref`
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, [see issue #57644 <https://github.com/rust-lang/rust/issues/57644>](https://github.com/rust-lang/rust/issues/57644)
note: `Ref` could refer to the variant defined here
  --> src/main.rs:12:5
   |
12 |     Ref { a: i32 },
   |     ^^^
note: `Ref` could also refer to the associated type defined here
  --> src/main.rs:3:5
   |
3  |     type Ref<'a>
   |     ^^^^^^^^^^^^
   = note: `#[deny(ambiguous_associated_items)]` on by default

The qualified paths do not work for named enum variants. However they work fine with unit and tuple enum variants.
I stumbled upon this while working on a proc. macro and this is now a blocker for proper functionality because there is no way to prevent the ambiguity.

If I apply the suggestion as shown by the Rust compiler on the playground I will get this error:

error[[E0658]](https://doc.rust-lang.org/stable/error_codes/E0658.html): usage of qualified paths in this context is experimental
  --> src/main.rs:26:32
   |
26 |             Self::Ref { a } => <Self as EnumRef>::Ref::Ref { a },
   |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^

But just test yourself via playground.
Note that if Test::Ref was a tuple struct enum variant this whole thing would work.

@TehPers
Copy link

TehPers commented Mar 26, 2023

@Robbepop If you're able to name Test instead of using Self, a temporary workaround is to define a type alias:

type SubRef<'lt> = <Test as EnumRef>::Ref<'lt>;
match self {
    Self::Ref { a } => SubRef::Ref { a },
}

Playground

@Robbepop
Copy link
Contributor

Robbepop commented Mar 26, 2023

@Robbepop If you're able to name Test instead of using Self, a temporary workaround is to define a type alias:

type SubRef<'lt> = <Test as EnumRef>::Ref<'lt>;
match self {
    Self::Ref { a } => SubRef::Ref { a },
}

Playground

Thanks for letting me know about this work around. Not sure if applicable in my proc. macro usage context since usually you want to avoid introducing identifiers at all costs in generated code.

edit: Furthermore getting this to work in a generic context where Test is generic over lifetimes, types or constants is even more pain to get working. I hope there will be a fix available soon for this bug.

edit 2: In case someone wants to toy around with the implemented fix. Here is the crate where I stumbled upon this bug: https://github.com/Robbepop/enum-ref

@WaffleLapkin
Copy link
Member

Well, <Tuple as Trait>::Assoc is a type, not a function or a value in general.
So it cannot be used in call expressions or path expressions, similarly to any other type.

@petrochenkov well, yes. But at the same time, let <Tuple as Trait>::Assoc(_) = todo!(); involves neither a function not a value (it's a pattern). So if we say that associated types reexport (of sorts) <Tuple as Trait>::Assoc { .. } kind of patterns, I don't really see why they shouldn't reexport other kinds of patterns.

I understand that it is a bit of a stretch, but to me it seems that S{}, T(), C are all referring to a constructor of sorts and the implementation via functions/constants is not very relevant here. Or, at least, I expect users to have a similar model, meaning that to them the restriction to S{} here would feel arbitrary/implementation driven.

@Alxandr
Copy link

Alxandr commented Aug 3, 2023

I have another use-case for this (the work around will also probably work though, so will look into that). I'm making html "components" using a jsx like syntax (from the rstml crate). The component trait will probably end up looking something like this:

pub trait Component: Sized {
  type Props: Into<Self> + Sized;

  fn into_template(self) -> HtmlTemplate;
}

This will be used when the user writes rstml like the following:

let template = html!(<MyComp foo="hello" baz=5 />);

which get's translated to something like this:

let template = <MyComp as Component>::Props { foo: "hello", baz: 5 }.into();

The reason for this is quite simple, while I can name the MyComp type, because the user entered it into the "html" - I don't know the name of the type for the props. It may not even be nameable. But, whereas the props is guaranteed to have all public fields so it's directly constructable, the component does not need to fulfill this constraint.

@kornelski
Copy link
Contributor

I've had a trait with an associated type:

trait Provider { type Config; }

and wanted to make an instance of the associated type (a struct) for a specific implementation (without importing the associated type by its concrete name, since that wasn't necessary in other contexts). I've tried using Foo::Config, but got E0223 with a suggestion:

help: use the fully-qualified path: <Foo as Provider>::Config

let config = <Foo as Provider>::Config { blah };
Foo::new(config);

A type alias worked (type Alias = <Foo as Provider>::Config;), so I'd be fine with either stabilizing this, or the E0223 error improved to to suggest a type alias when necessary.

@cart
Copy link

cart commented Nov 21, 2023

Also hit this while implementing a jsx-like bsn for enums in Bevy:

let mut #type_ident = <<#path as #bevy_scene::Schematic>::Props as Default>::default();
if let <#path as #bevy_scene::Schematic>::Props::#variant { #(#fields,)* .. } = &mut #type_ident {
    #(#variant_props)*
}

The actual type of #path as #bevy_scene::Schematic>::Props is not known by the bsn macro.

The "type alias" workaround does work.

@fmease fmease added the F-more_qualified_paths `#![feature(more_qualified_paths)]` label Feb 7, 2024
nwn added a commit to nwn/ros2_rust that referenced this issue Sep 28, 2024
This adds methods to ActionImpl for creating and accessing
action-specific message types. These are needed by the rclrs
ActionClient to generically read and write RMW messages.

Due to issues with qualified paths in certain places
(rust-lang/rust#86935), the generator now
refers directly to its service and message types rather than going
through associated types of the various traits. This also makes the
generated code a little easier to read, with the trait method signatures
from rosidl_runtime_rs still enforcing type-safety.
nwn added a commit to nwn/ros2_rust that referenced this issue Sep 28, 2024
This adds methods to ActionImpl for creating and accessing
action-specific message types. These are needed by the rclrs
ActionClient to generically read and write RMW messages.

Due to issues with qualified paths in certain places
(rust-lang/rust#86935), the generator now
refers directly to its service and message types rather than going
through associated types of the various traits. This also makes the
generated code a little easier to read, with the trait method signatures
from rosidl_runtime_rs still enforcing type-safety.
nwn added a commit to nwn/ros2_rust that referenced this issue Sep 28, 2024
This adds methods to ActionImpl for creating and accessing
action-specific message types. These are needed by the rclrs
ActionClient to generically read and write RMW messages.

Due to issues with qualified paths in certain places
(rust-lang/rust#86935), the generator now
refers directly to its service and message types rather than going
through associated types of the various traits. This also makes the
generated code a little easier to read, with the trait method signatures
from rosidl_runtime_rs still enforcing type-safety.
nwn added a commit to nwn/ros2_rust that referenced this issue Sep 28, 2024
This adds methods to ActionImpl for creating and accessing
action-specific message types. These are needed by the rclrs
ActionClient to generically read and write RMW messages.

Due to issues with qualified paths in certain places
(rust-lang/rust#86935), the generator now
refers directly to its service and message types rather than going
through associated types of the various traits. This also makes the
generated code a little easier to read, with the trait method signatures
from rosidl_runtime_rs still enforcing type-safety.
maspe36 pushed a commit to ros2-rust/ros2_rust that referenced this issue Oct 26, 2024
* Added action template

* Added action generation

* Added basic create_action_client function

* dded action generation

* checkin

* Fix missing exported pre_field_serde field

* Removed extra code

* Sketch out action server construction and destruction

This follows generally the same pattern as the service server. It
required adding a typesupport function to the Action trait and pulling
in some more rcl_action bindings.

* Fix action typesupport function

* Add ActionImpl trait with internal messages and services

This is incomplete, since the service types aren't yet being generated.

* Split srv.rs.em into idiomatic and rmw template files

This results in the exact same file being produced for services,
except for some whitespace changes. However, it enables actions to
invoke the respective service template for its generation, similar to
how the it works for services and their underlying messages.

* Generate underlying service definitions for actions

Not tested

* Add runtime trait to get the UUID from a goal request

C++ uses duck typing for this, knowing that for any `Action`, the type
`Action::Impl::SendGoalService::Request` will always have a `goal_id`
field of type `unique_identifier_msgs::msg::UUID` without having to
prove this to the compiler. Rust's generics are more strict, requiring
that this be proven using type bounds.

The `Request` type is also action-specific as it contains a `goal` field
containing the `Goal` message type of the action. We therefore cannot
enforce that all `Request`s are a specific type in `rclrs`.

This seems most easily represented using associated type bounds on the
`SendGoalService` associated type within `ActionImpl`. To avoid
introducing to `rosidl_runtime_rs` a circular dependency on message
packages like `unique_identifier_msgs`, the `ExtractUuid` trait only
operates on a byte array rather than a more nicely typed `UUID` message
type.

I'll likely revisit this as we introduce more similar bounds on the
generated types.

* Integrate RMW message methods into ActionImpl

Rather than having a bunch of standalone traits implementing various
message functions like `ExtractUuid` and `SetAccepted`, with the
trait bounds on each associated type in `ActionImpl`, we'll instead add
these functions directly to the `ActionImpl` trait. This is simpler on
both the rosidl_runtime_rs and the rclrs side.

* Add rosidl_runtime_rs::ActionImpl::create_feedback_message()

Adds a trait method to create a feedback message given the goal ID and
the user-facing feedback message type. Depending on how many times we do
this, it may end up valuable to define a GoalUuid type in
rosidl_runtime_rs itself. We wouldn't be able to utilize the
`RCL_ACTION_UUID_SIZE` constant imported from `rcl_action`, but this is
pretty much guaranteed to be 16 forever.

Defining this method signature also required inverting the super-trait
relationship between Action and ActionImpl. Now ActionImpl is the
sub-trait as this gives it access to all of Action's associated types.
Action doesn't need to care about anything from ActionImpl (hopefully).

* Add GetResultService methods to ActionImpl

* Implement ActionImpl trait methods in generator

These still don't build without errors, but it's close.

* Replace set_result_response_status with create_result_response

rclrs needs to be able to generically construct result responses,
including the user-defined result field.

* Implement client-side trait methods for action messages

This adds methods to ActionImpl for creating and accessing
action-specific message types. These are needed by the rclrs
ActionClient to generically read and write RMW messages.

Due to issues with qualified paths in certain places
(rust-lang/rust#86935), the generator now
refers directly to its service and message types rather than going
through associated types of the various traits. This also makes the
generated code a little easier to read, with the trait method signatures
from rosidl_runtime_rs still enforcing type-safety.

* Format the rosidl_runtime_rs::ActionImpl trait

* Wrap longs lines in rosidl_generator_rs action.rs

This at least makes the template easier to read, but also helps with the
generated code. In general, the generated code could actually fit on one
line for the function signatures, but it's not a big deal to split it
across multiple.

* Use idiomatic message types in Action trait

This is user-facing and so should use the friendly message types.

* Cleanup ActionImpl using type aliases

Signed-off-by: Michael X. Grey <grey@openrobotics.org>

* Formatting

* Switch from std::os::raw::c_void to std::ffi::c_void

While these are aliases of each other, we might as well use the more
appropriate std::ffi version, as requested by reviewers.

* Clean up rosidl_generator_rs's cmake files

Some of the variables are present but no longer used. Others were not
updated with the action changes.

* Add a short doc page on the message generation pipeline

This should help newcomers orient themselves around the rosidl_*_rs
packages.

---------

Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Co-authored-by: Esteve Fernandez <esteve@apache.org>
Co-authored-by: Michael X. Grey <grey@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) A-parser Area: The parsing of Rust source code to an AST C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-more_qualified_paths `#![feature(more_qualified_paths)]` S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests