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

Continue renaming the comparison traits #14534

Merged
merged 3 commits into from
May 31, 2014
Merged

Conversation

alexcrichton
Copy link
Member

This is part 2 of the saga of renaming the Partial/Total equality and comparison traits.

@@ -1436,7 +1436,7 @@ trait Circle : Shape { fn radius() -> f64; }
~~~~

the syntax `Circle : Shape` means that types that implement `Circle` must also have an implementation for `Shape`.
Multiple supertraits are separated by `+`, `trait Circle : Shape + Eq { }`.
Multiple supertraits are separated by `+`, `trait Circle : Shape + PartialEq { }`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave this with Eq

@alexcrichton
Copy link
Member Author

I agree that many of these renamings should not be PartialEq, but should rather be Eq. I wanted to do this in a methodical fashion to be uniform across the board, and then I plan on going back over the codebase after the TotalEq rename and move everything back to Eq/Ord wherever possible.

@alexcrichton
Copy link
Member Author

I could also do that as part of this PR, but I was afraid of the bootstrapping concerns because Eq basically doesn't exist at stage0.

@sfackler
Copy link
Member

@alexcrichton
Copy link
Member Author

Tidy issues resolved.

This is part of the ongoing renaming of the equality traits. See rust-lang#12517 for more
details. All code using Eq/Ord will temporarily need to move to Partial{Eq,Ord}
or the Total{Eq,Ord} traits. The Total traits will soon be renamed to {Eq,Ord}.

cc rust-lang#12517

[breaking-change]
This commit adds the groundwork for the renaming of the Total{Eq,Ord} traits.
After this commit hits a snapshot, the traits can be renamed.
@sfackler
Copy link
Member

What's the timeline on moving things back to Eq/Ord?

@alexcrichton
Copy link
Member Author

As soon as this hits a snapshot I will rename to Eq/Ord

bors added a commit that referenced this pull request May 31, 2014
This is part 2 of the saga of renaming the Partial/Total equality and comparison traits.
@bors bors closed this May 31, 2014
@bors bors merged commit bb96ee6 into rust-lang:master May 31, 2014
@alexcrichton alexcrichton deleted the snapshots branch May 31, 2014 06:33
bors added a commit that referenced this pull request Jun 1, 2014
This completes the last stage of the renaming of the comparison hierarchy of
traits. This change renames TotalEq to Eq and TotalOrd to Ord.

In the future the new Eq/Ord will be filled out with their appropriate methods,
but for now this change is purely a renaming change.

This continues the work of #12517, continuing the work in #14534. This patch accomplishes the final rename of `TotalEq` to `TotalOrd`. I wanted to get this patch landed ASAP so we don't have to deal much with "where did `Eq` and `Ord` go?"

I have yet to do another pruning pass over the compiler to change all usage of `PartialEq` to `Eq` where appropriate. I will do this soon as well.
lnicola pushed a commit to lnicola/rust that referenced this pull request Jun 19, 2023
…anic, r=Veykril

fix: derive source scope from syntax node to be transformed

Fixes rust-lang#14534

When we use `PathTransform` for associated items of a trait, we have been feeding `SemanticsScope` for the trait definition to it as source scope. `PathTransform` uses the source scope to resolve paths in associated items to find which path to transform. In the course of path resolution, the scope is responsible for lowering `ast::MacroType`s (because they can be written within a path) using `AstIdMap` for the scope's `HirFileId`.

The problem here is that when an associated item is generated by a macro, the scope for the trait is different from the scope for that associated item. The former can only resolve the top-level macros within the trait definition but not the macro calls generated by those top-level macros. We need the latter to resolve such nested macros.

This PR makes sure that we pass `SemanticsScope` for each associated item we're applying path transformation to.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants