Skip to content

Remove ordering traits from LocalExpnId #92358

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed

Conversation

pierwill
Copy link
Member

See #90317.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 28, 2021
@rust-highfive
Copy link
Contributor

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 28, 2021
@pierwill pierwill marked this pull request as draft December 28, 2021 16:15
@pierwill
Copy link
Member Author

@rustbot author

@rustbot rustbot 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 Dec 28, 2021
@rust-log-analyzer

This comment has been minimized.

@pierwill
Copy link
Member Author

r? @cjgillot

@cjgillot
Copy link
Contributor

I think we want to keep the Idx impl for LocalExpnId. IndexVec is quite a good data structure for an interner. It may be easier to make the ordering traits opt-out in newtype_index macro, like we do with encodable/decodable traits.

@pierwill
Copy link
Member Author

It may be easier to make the ordering traits opt-out in newtype_index macro, like we do with encodable/decodable traits.

Ah, great! I was wondering if something like that was possible. I'll check out that code. 👍

@pierwill pierwill closed this Dec 28, 2021
@pierwill pierwill force-pushed the untrack-localexpnid-90317-2 branch from 9162bc9 to 442248d Compare December 28, 2021 18:59
@pierwill pierwill reopened this Dec 28, 2021
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking chalk-ir v0.75.0
    Checking tracing v0.1.28
    Checking tracing-subscriber v0.3.3
    Checking rustc_index v0.0.0 (/checkout/compiler/rustc_index)
error[E0277]: can't compare `MyIdx` with `MyIdx`
    |
69  |     / macro_rules! newtype_index {
70  |     |     // ---- public rules ----
71  |     |
---
94  | |   |                           $($tokens)+);
    | |___|______________________________________- in this macro invocation (#2)
...       |
208 |     |         impl ::std::iter::Step for $type {
    |     |              ^^^^^^^^^^^^^^^^^ no implementation for `MyIdx < MyIdx` and `MyIdx > MyIdx`
386 | /   |         $crate::newtype_index!(
387 | |   |             @derives      []
388 | |   |             @attrs        [$(#[$attrs])*]
389 | |   |             @type         [$type]
389 | |   |             @type         [$type]
...   |   |
392 | |   |             @debug_format [$debug_format]
393 | |   |                           $($tokens)*);
    | |___|______________________________________- in this macro invocation (#3)
...       |
418 | /   |         $crate::newtype_index!(
419 | |   |             @derives      [$($derives,)*]
420 | |   |             @attrs        [$(#[$attrs])*]
421 | |   |             @type         [$type]
...   |   |
424 | |   |             @debug_format [$debug_format]
425 | |   |                           $name = $constant,);
    | |___|_____________________________________________- in this macro invocation (#4)
456 | /   |         $crate::newtype_index!(
457 | |   |             @derives      [$($derives,)*]
458 | |   |             @attrs        [$(#[$attrs])*]
459 | |   |             @type         [$type]
---
    |       in this expansion of `$crate::newtype_index!` (#5)
    |
   ::: compiler/rustc_index/src/vec/tests.rs:2:1
    |
2   |       newtype_index!(struct MyIdx { MAX = 0xFFFF_FFFA });
    |
    |
    = help: the trait `PartialOrd` is not implemented for `MyIdx`
note: required by a bound in `Step`
    |
    |
24  | pub trait Step: Clone + PartialOrd + Sized {
    |                         ^^^^^^^^^^ required by this bound in `Step`

error[E0277]: can't compare `MyIdx` with `MyIdx`
    |
69  |     / macro_rules! newtype_index {
70  |     |     // ---- public rules ----
71  |     |
---
93  | |   |             @debug_format ["{}"]
94  | |   |                           $($tokens)+);
    | |___|______________________________________- in this macro invocation (#2)
...       |
229 |     |         unsafe impl ::std::iter::TrustedStep for $type {}
    |     |                     ^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `MyIdx < MyIdx` and `MyIdx > MyIdx`
386 | /   |         $crate::newtype_index!(
387 | |   |             @derives      []
388 | |   |             @attrs        [$(#[$attrs])*]
389 | |   |             @type         [$type]
389 | |   |             @type         [$type]
...   |   |
392 | |   |             @debug_format [$debug_format]
393 | |   |                           $($tokens)*);
    | |___|______________________________________- in this macro invocation (#3)
...       |
418 | /   |         $crate::newtype_index!(
419 | |   |             @derives      [$($derives,)*]
420 | |   |             @attrs        [$(#[$attrs])*]
421 | |   |             @type         [$type]
...   |   |
424 | |   |             @debug_format [$debug_format]
425 | |   |                           $name = $constant,);
    | |___|_____________________________________________- in this macro invocation (#4)
456 | /   |         $crate::newtype_index!(
457 | |   |             @derives      [$($derives,)*]
458 | |   |             @attrs        [$(#[$attrs])*]
459 | |   |             @type         [$type]
---
    |       in this expansion of `$crate::newtype_index!` (#5)
    |
   ::: compiler/rustc_index/src/vec/tests.rs:2:1
    |
2   |       newtype_index!(struct MyIdx { MAX = 0xFFFF_FFFA });
    |
    |
    = help: the trait `PartialOrd` is not implemented for `MyIdx`
note: required by a bound in `TrustedStep`
    |
    |
74  | pub unsafe trait TrustedStep: Step {}
    |                               ^^^^ required by this bound in `TrustedStep`
For more information about this error, try `rustc --explain E0277`.
error: could not compile `rustc_index` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error: build failed

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking chalk-ir v0.75.0
    Checking tracing v0.1.28
    Checking tracing-subscriber v0.3.3
    Checking rustc_index v0.0.0 (/checkout/compiler/rustc_index)
error[E0277]: can't compare `MyIdx` with `MyIdx`
    |
69  |     / macro_rules! newtype_index {
70  |     |     // ---- public rules ----
71  |     |
---
94  | |   |                           $($tokens)+);
    | |___|______________________________________- in this macro invocation (#2)
...       |
208 |     |         impl ::std::iter::Step for $type {
    |     |              ^^^^^^^^^^^^^^^^^ no implementation for `MyIdx < MyIdx` and `MyIdx > MyIdx`
386 | /   |         $crate::newtype_index!(
387 | |   |             @derives      []
388 | |   |             @attrs        [$(#[$attrs])*]
389 | |   |             @type         [$type]
389 | |   |             @type         [$type]
...   |   |
392 | |   |             @debug_format [$debug_format]
393 | |   |                           $($tokens)*);
    | |___|______________________________________- in this macro invocation (#3)
...       |
418 | /   |         $crate::newtype_index!(
419 | |   |             @derives      [$($derives,)*]
420 | |   |             @attrs        [$(#[$attrs])*]
421 | |   |             @type         [$type]
...   |   |
424 | |   |             @debug_format [$debug_format]
425 | |   |                           $name = $constant,);
    | |___|_____________________________________________- in this macro invocation (#4)
456 | /   |         $crate::newtype_index!(
457 | |   |             @derives      [$($derives,)*]
458 | |   |             @attrs        [$(#[$attrs])*]
459 | |   |             @type         [$type]
---
    |       in this expansion of `$crate::newtype_index!` (#5)
    |
   ::: compiler/rustc_index/src/vec/tests.rs:2:1
    |
2   |       newtype_index!(struct MyIdx { MAX = 0xFFFF_FFFA });
    |
    |
    = help: the trait `PartialOrd` is not implemented for `MyIdx`
note: required by a bound in `Step`
    |
    |
24  | pub trait Step: Clone + PartialOrd + Sized {
    |                         ^^^^^^^^^^ required by this bound in `Step`

error[E0277]: can't compare `MyIdx` with `MyIdx`
    |
69  |     / macro_rules! newtype_index {
70  |     |     // ---- public rules ----
71  |     |
---
93  | |   |             @debug_format ["{}"]
94  | |   |                           $($tokens)+);
    | |___|______________________________________- in this macro invocation (#2)
...       |
229 |     |         unsafe impl ::std::iter::TrustedStep for $type {}
    |     |                     ^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `MyIdx < MyIdx` and `MyIdx > MyIdx`
386 | /   |         $crate::newtype_index!(
387 | |   |             @derives      []
388 | |   |             @attrs        [$(#[$attrs])*]
389 | |   |             @type         [$type]
389 | |   |             @type         [$type]
...   |   |
392 | |   |             @debug_format [$debug_format]
393 | |   |                           $($tokens)*);
    | |___|______________________________________- in this macro invocation (#3)
...       |
418 | /   |         $crate::newtype_index!(
419 | |   |             @derives      [$($derives,)*]
420 | |   |             @attrs        [$(#[$attrs])*]
421 | |   |             @type         [$type]
...   |   |
424 | |   |             @debug_format [$debug_format]
425 | |   |                           $name = $constant,);
    | |___|_____________________________________________- in this macro invocation (#4)
456 | /   |         $crate::newtype_index!(
457 | |   |             @derives      [$($derives,)*]
458 | |   |             @attrs        [$(#[$attrs])*]
459 | |   |             @type         [$type]
---
    |       in this expansion of `$crate::newtype_index!` (#5)
    |
   ::: compiler/rustc_index/src/vec/tests.rs:2:1
    |
2   |       newtype_index!(struct MyIdx { MAX = 0xFFFF_FFFA });
    |
    |
    = help: the trait `PartialOrd` is not implemented for `MyIdx`
note: required by a bound in `TrustedStep`
    |
    |
74  | pub unsafe trait TrustedStep: Step {}
    |                               ^^^^ required by this bound in `TrustedStep`
For more information about this error, try `rustc --explain E0277`.
error: could not compile `rustc_index` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error: build failed

@pierwill pierwill changed the title [WIP] Remove ordering traits from LocalExpnId Remove ordering traits from LocalExpnId Dec 30, 2021
@pierwill
Copy link
Member Author

pierwill commented Jan 5, 2022

It may be easier to make the ordering traits opt-out in newtype_index macro, like we do with encodable/decodable traits.

@cjgillot Could you offer a clue on how to do this, or an example from the encodable traits you mention? I'm a macro newbie!

@pierwill
Copy link
Member Author

Closing this for now. Removing these traits will depend on #93878.

@pierwill pierwill closed this Feb 15, 2022
@pierwill pierwill deleted the untrack-localexpnid-90317-2 branch February 15, 2022 17:51
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants