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

Rustfmt deletes comments in first position of trait bound #6051

Open
sffc opened this issue Jan 31, 2024 · 4 comments
Open

Rustfmt deletes comments in first position of trait bound #6051

sffc opened this issue Jan 31, 2024 · 4 comments
Labels
a-comments bug Panic, non-idempotency, invalid code, etc.

Comments

@sffc
Copy link

sffc commented Jan 31, 2024

Example code:

    pub fn try_new_with_date_length_unstable<P>(
        provider: &P,
        locale: &DataLocale,
        length: length::Date,
    ) -> Result<Self, Error>
    where
        P: // Explanatory comment about the bounds
            ?Sized
            + DataProvider<BuddhistDatePatternV1Marker>
            + DataProvider<BuddhistYearNamesV1Marker>
            + DataProvider<BuddhistMonthNamesV1Marker>
    { ... }

Running this through rustfmt deletes the // Explanatory comment about the bounds.

Workaround: move the ?Sized into the first position. If your bound is not ?Sized, you can use Sized. The comment is retained:

    pub fn try_new_with_date_length_unstable<P>(
        provider: &P,
        locale: &DataLocale,
        length: length::Date,
    ) -> Result<Self, Error>
    where
        P: ?Sized
            // Explanatory comment about the bounds
            + DataProvider<BuddhistDatePatternV1Marker>
            + DataProvider<BuddhistYearNamesV1Marker>
            + DataProvider<BuddhistMonthNamesV1Marker>
    { ... }

Possibly related to #3669, #4666, #5059 but the examples are in different contexts so I thought I would file a new issue with this test case.

CC @Manishearth

@ytmimi ytmimi added bug Panic, non-idempotency, invalid code, etc. a-comments labels Feb 1, 2024
@nnethercote
Copy link
Contributor

I just hit this:

diff --git a/tests/rustdoc/rfc-2632-const-trait-impl.rs b/tests/rustdoc/rfc-2632-const-trait-impl.rs
index 6f264969e54..0fec059f951 100644
--- a/tests/rustdoc/rfc-2632-const-trait-impl.rs
+++ b/tests/rustdoc/rfc-2632-const-trait-impl.rs
@@ -24,9 +24,9 @@ pub trait Tr<T> {
     // @has - '//section[@id="method.a"]/h4[@class="code-header"]/a[@class="trait"]' 'Fn'
     // @!has - '//section[@id="method.a"]/h4[@class="code-header"]/span[@class="where"]' '~const'
     // @has - '//section[@id="method.a"]/h4[@class="code-header"]/div[@class="where"]' ': Fn'
-    fn a<A: /* ~const */ Fn() + ~const Destruct>()
+    fn a<A: Fn() + ~const Destruct>()
     where
-        Option<A>: /* ~const */ Fn() + ~const Destruct,
+        Option<A>: Fn() + ~const Destruct,
     {
     }
 }

I have a comment removed in both trait bounds: within the generic params, and in the where clause.

@ijackson
Copy link

Another repro:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f590ab120d35f6aa1d296e9da5ce9d0f

@rustbot label +C-bug
@rustbot label +p-high

Not sure Rustbot will let me set the p-high label, but I feel I should try. After all this is a case where rustfmt corrupts the code, lossily, in a way that is unlikely to be detected.

@rustbot
Copy link
Collaborator

rustbot commented Jul 18, 2024

Error: The feature relabel is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

The feature relabel is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@RReverser
Copy link
Contributor

I think hitting the same issue, so not going to raise a new one, but happens for me even in non-first position.

Minimal repro passed via rustfmt --check:

-trait RegistrableDevice<DynTrait: ?Sized>: Sized /* + Unsize<DynTrait> */ {}
+trait RegistrableDevice<DynTrait: ?Sized>: Sized {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-comments bug Panic, non-idempotency, invalid code, etc.
Projects
None yet
Development

No branches or pull requests

6 participants