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

Don't remove const generic when using #[feature(generic_const_items)] #5996

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ytmimi
Copy link
Contributor

@ytmimi ytmimi commented Dec 29, 2023

Fixes #5995

Added support for rewriting generics on const items.

Fixes 5995

Added support for rewriting generics on const items.
@fmease
Copy link
Member

fmease commented Feb 8, 2024

Could you add a test for where-clauses, too? That would be great! Thanks in advance :)

@ytmimi
Copy link
Contributor Author

ytmimi commented Feb 8, 2024

@fmease happy to add the test case, but would you mind providing the code snippet. I'm not as familiar with this feature as you are.

@fmease
Copy link
Member

fmease commented Feb 8, 2024

Sure, for example:

pub const K<T>: Option<T> = None
where
    String: From<T>;

And:

pub trait Trait<T: ?Sized> {
    const C<'a>: &'a T
    where
        T: 'a + Eq;
}

@fmease
Copy link
Member

fmease commented Feb 8, 2024

Note that generic const items don't have an entry inside the Style Guide yet as you might or might not have noticed. However, the formatting should basically follow the new1 formatting of type aliases and associated types (except that they don't need to deal with legacy-style leading where clauses, cc #5887).

Footnotes

  1. Proposal: Change syntax of where clauses on type aliases rust#89122

@ytmimi
Copy link
Contributor Author

ytmimi commented Feb 9, 2024

Looks like there's still a little work to be done. Formatting with this branch on the snippets you provided removes the where clauses:

input:

pub const K<T>: Option<T> = None
where
    String: From<T>;

pub trait Trait<T: ?Sized> {
    const C<'a>: &'a T
    where
        T: 'a + Eq;
}

output:

pub const K<T>: Option<T> = None;

pub trait Trait<T: ?Sized> {
    const C<'a>: &'a T;
}

@fmease
Copy link
Member

fmease commented Feb 9, 2024

Yeah, seems to need rewrite_where_clause somewhere.

@ytmimi
Copy link
Contributor Author

ytmimi commented Feb 9, 2024

Just double checking, but where clauses are new to const items now that they can be generic with the #[feature(generic_const_items)] , right?

@fmease
Copy link
Member

fmease commented Feb 9, 2024

Yes, they are part of generic_const_items and don't exist outside of the feature.

@ytmimi
Copy link
Contributor Author

ytmimi commented Feb 17, 2024

Note to self, unlike the deprecated type alias syntax, it's not possible to place the where clause before the assignment. I put together this small snippet that compiles:

#![feature(generic_const_items)]

pub const K<T>: Option<T> = None
where
    String: From<T>,
    T: std::fmt::Debug;


#[derive(Debug)]
struct ToString;

impl From<ToString> for String {
    fn from(_value: ToString) -> Self {
        String::new()
    }
}

fn main() {
    println!("{:?}", K::<ToString>);
}

But when I switch the order of the where clause and the assignment I get the following compilation errors:

#![feature(generic_const_items)]

pub const K<T>: Option<T>
where
    String: From<T>,
    T: std::fmt::Debug,
 = None;

#[derive(Debug)]
struct ToString;

impl From<ToString> for String {
    fn from(_value: ToString) -> Self {
        String::new()
    }
}

fn main() {
    println!("{:?}", K::<ToString>);
}
error: where clauses are not allowed before const item bodies
 --> example.rs:5:1
  |
4 |   pub const K<T>: Option<T>
  |             - while parsing this const item
5 | / where
6 | |     String: From<T>,
7 | |     T: std::fmt::Debug,
  | |_______________________^ unexpected where clause
8 |   = None;
  |     ---- the item body
  |
help: move the body before the where clause
  |
5 ~ = None where
6 |     String: From<T>,
7 ~     T: std::fmt::Debug,;
  |

error: aborting due to 1 previous error

@zesterer
Copy link

zesterer commented Sep 5, 2024

Is there a plan for this to be merged? I've just hit this case myself.

@ytmimi
Copy link
Contributor Author

ytmimi commented Sep 5, 2024

@zesterer I haven't had a chance to revisit this and add the code needed to retaining the where clause.

In the meantime you might work around the issue by adding a #[rustfmt::skip] annotation to the generic const.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generic_const_items syntax gets deleted
4 participants