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

Stability annotation on defaulted generics fails #79499

Open
TimDiekmann opened this issue Nov 28, 2020 · 2 comments
Open

Stability annotation on defaulted generics fails #79499

TimDiekmann opened this issue Nov 28, 2020 · 2 comments
Labels
A-allocators Area: Custom and system allocators A-stability Area: `#[stable]`, `#[unstable]` etc. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@TimDiekmann
Copy link
Member

I'm adding <A: AllocRef = Global> to string types. Strings don't have a generic parameter, so the defaulted generic parameter is the first parameter:

pub struct String<#[unstable(feature = "allocator_api", issue = "32838")] A: AllocRef = Global> {}

However, the compiler complains, that this annotation is "useless":

error: This stability annotation is useless
   --> library/alloc/src/string.rs:280:75
    |
280 | pub struct String<#[unstable(feature = "allocator_api", issue = "32838")] A: AllocRef = Global> {
    |                                                                           ^

Was this simply not added by #77118 or is this a false positive in general?

cc @varkor @Avi-D-coder

@TimDiekmann TimDiekmann added the C-bug Category: This is a bug. label Nov 28, 2020
@jonas-schievink jonas-schievink added A-stability Area: `#[stable]`, `#[unstable]` etc. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-allocators Area: Custom and system allocators labels Nov 28, 2020
@varkor
Copy link
Member

varkor commented Nov 28, 2020

The error is occurring here, for reference:

// Error if prohibited, or can't inherit anything from a container.
if kind == AnnotationKind::Prohibited
|| (kind == AnnotationKind::Container && stab.level.is_stable() && is_deprecated)
{
self.tcx.sess.span_err(item_sp, "This stability annotation is useless");
}

Off the top of my head, I'm not sure which part of the condition is satisfied. There's no reason I see to expect this not to work, so it could well have been a case overlooked in #77118.

@Avi-D-coder: would you be interested in taking a look to see what's going wrong? I should be able to give more immediate response than last time (it'd just take me a little while to find time to debug, myself) :)

@zachs18
Copy link
Contributor

zachs18 commented Sep 6, 2022

If I remove derive(PartialOrd, Eq, Ord) from struct String (i.e. manually implement them, like PartialEq already was), then I don't get the This stability annotation is useless error anymore, so I think the error might be coming from a derive macro instead of the struct declaration itself.


Specifically, for this code (outside of stdlib, because IDK how to cargo expand inside stdlib)

#![feature(allocator_api)]
#![feature(std_internals)]

use std::alloc::{Allocator, Global};

#[derive(PartialOrd, Eq, Ord)]
pub struct String<#[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global> {
    vec: Vec<u8, A>,
}
impl<A: Allocator, B: Allocator> PartialEq<String<B>> for String<A> {
    fn eq(&self, other: &String<B>) -> bool { todo!() }
}

cargo +nightly expand gives

// error "stability attributes may not be used outside of the standard library" ignored
// .. etc
impl<#[unstable(feature = "allocator_api", issue = "32838")] A: Allocator>
    ::core::marker::StructuralEq for String<A>
{
}
// .. etc (None of the other impls have the stability annotation)

and I think this impl is what's causing the error; it just looks like its the struct definition because the Eq derive macro copies the span from the struct definition.


If this is correct, maybe this issue should be renamed to something like "Eq derive macro copies stability annotation on generics into impl StructuralEq, causing compilation error"?

(Either way, if String (/CString/etc) is made allocator-aware, it would be better to manually implement (Partial){Eq,Ord} anyway, so as not to have an <A: Allocator + (Partial){Eq,Ord}> bound.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-allocators Area: Custom and system allocators A-stability Area: `#[stable]`, `#[unstable]` etc. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants