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

WIP Generic associated types in trait paths #78978

Closed
wants to merge 5 commits into from

Conversation

b-naber
Copy link
Contributor

@b-naber b-naber commented Nov 12, 2020

Is supposed to fix #67510, #68648, #68649, #68650, #68652, #74684

@petrochenkov Asked me to open a WIP PR, so he can look at the part that was changed in the parser.

A couple of potential problems and things to work on are:

  • Error messages must probably be improved in the parser
  • There are a couple of spans that are not completely correct
  • There are a couple of conflicts with error messages that were created for const-generics. E.g. in S::<5 + 2 >> 7>;, which suggests that const expressions must be enclosed in braces, whereas with the current changes we get: expected one of ,or>, found +
  • Currently in parse_assoc_equality_term we return Ok(mk_ty(..., TyKind::Error)) unless the generic argument that is parsed in the beginning of the function is of type GenericArg::Type, but a TyKind::Error result is currently not handled after the call.
  • In bounds_reference_self in object safety checking the map call that used subst_supertraitwas removed, I've talked to both @matthewjasper and @nikomatsakis about this and both didn't know why the call was there. All relevant ui tests still pass after the removal.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 12, 2020
@lcnr
Copy link
Contributor

lcnr commented Nov 12, 2020

r? @petrochenkov for now

@rust-highfive rust-highfive assigned petrochenkov and unassigned lcnr Nov 12, 2020
@matthewjasper matthewjasper self-assigned this Nov 12, 2020
@petrochenkov
Copy link
Contributor

@b-naber
Meta: could you split the frontend part of this PR (from parsing to AST lowering) to a separate PR, so we could land this piece-wise instead of passing a huge PR from one reviewer to another.
AST lowering can produce a "this is not yet implemented" error in the initial PR.

@petrochenkov
Copy link
Contributor

Meta 2: the diagnostics efforts in the parser can be deferred to a separate PR to make this PR smaller and more digestible so we can be sure that the core logic is correct.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 13, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2020
…ochenkov

Generic Associated Types in Trait Paths - Ast part

The Ast part of rust-lang#78978

r? `@petrochenkov`
@bors
Copy link
Contributor

bors commented Nov 27, 2020

☔ The latest upstream changes (presumably #79266) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@petrochenkov
Copy link
Contributor

petrochenkov commented Nov 30, 2020

Not sure whether this PR is fully superseded by #79554 or not, but I'm going to mark it as blocked on it.

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 30, 2020
@b-naber b-naber closed this Dec 1, 2020
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Feb 5, 2021
…-trait-paths, r=jackh726

Generic associated types in trait paths

This is the second part of rust-lang#78978

This should fix:

Fixes rust-lang#67510
Fixes rust-lang#68648
Fixes rust-lang#68649
Fixes rust-lang#68650
Fixes rust-lang#68652
Fixes rust-lang#74684
Fixes rust-lang#76535
Fixes rust-lang#79422
Fixes rust-lang#80433

and implement the remaining functionality needed for rust-lang#44265

r? `@matthewjasper`
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Feb 5, 2021
…-trait-paths, r=jackh726

Generic associated types in trait paths

This is the second part of rust-lang#78978

This should fix:

Fixes rust-lang#67510
Fixes rust-lang#68648
Fixes rust-lang#68649
Fixes rust-lang#68650
Fixes rust-lang#68652
Fixes rust-lang#74684
Fixes rust-lang#76535
Fixes rust-lang#79422
Fixes rust-lang#80433

and implement the remaining functionality needed for rust-lang#44265

r? ``@matthewjasper``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: Blocked on something else such as an RFC or other implementation work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow generic associate types in trait paths
6 participants