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

Add support for stability attributes: #[deprecated], #[experimental], etc. #8921

Closed
wants to merge 1 commit into from

Conversation

huonw
Copy link
Member

@huonw huonw commented Sep 1, 2013

Significant progress on #6875, enough that I'll open new bugs and turn that into a metabug when this lands.

Description & example in the commit message.

There are 6 new compiler recognised attributes: deprecated, experimental,
unstable, stable, frozen, locked (these levels are taken directly from
Node's "stability index"[1]). These indicate the stability of the
item to which they are attached; e.g. `#[deprecated] fn foo() { .. }`
says that `foo` is deprecated.

This comes with 3 lints for the first 3 levels (with matching names) that
will detect the use of items marked with them (the `unstable` lint
includes items with no stability attribute). The attributes can be given
a short text note that will be displayed by the lint. An example:

    #[warn(unstable)]; // `allow` by default

    #[deprecated="use `bar`"]
    fn foo() { }

    #[stable]
    fn bar() { }

    fn baz() { }

    fn main() {
        foo(); // "warning: use of deprecated item: use `bar`"

        bar(); // all fine

        baz(); // "warning: use of unmarked item"
    }

The lints currently only check the "edges" of the AST: i.e. functions,
methods[2], structs and enum variants. Any stability attributes on modules,
enums, traits and impls are not checked.

[1]: http://nodejs.org/api/documentation.html
[2]: the method check is currently incorrect and doesn't work.
@huonw
Copy link
Member Author

huonw commented Sep 3, 2013

There's one major problem: I can't get the attributes attached to a method from the point that it is called. i.e.

struct Foo;
impl Foo {
    #[deprecated]
    fn method(&self) {}
}

fn main() {
    Foo.method(); // how to I retrieve the `deprecated` attribute above?
}

The information isn't in the def_map and I can't get access to the middle::typeck::method_map in middle::lint.

@brson
Copy link
Contributor

brson commented Sep 3, 2013

1000 kudos!

@brson
Copy link
Contributor

brson commented Sep 3, 2013

Looks like method_map is the way to get that info, so I guess it needs to be given to the lint passes.

@brson
Copy link
Contributor

brson commented Sep 3, 2013

I think this is a crucial feature for stabilizing the language. Some considerations

  • We will want to detect non-leaves too. For example, the name of the std::vec module is not going to change, so that's one of the easiest things in std to mark stable. It's not clear whether there needs to be inheritance of stability levels or not.
  • Once this is fully baked we'll want to turn it on by default so that users know which unstable std and extra API's they are using. How do we do this without making it obnoxious to use their own crates? In other words, we'll put in the effort to mark up std and extra, but we don't want to force users to put #[stable] attrs all over their own work, or get a bunch of false positives by linking to their own crates.
  • Similarly to above, what happens when using unstable features from within the same crate?

@brson
Copy link
Contributor

brson commented Sep 3, 2013

I suspect the term 'unmarked' is a bit unclear here. Maybe also call it unstable but with some additional note that the item isn't annotated.

bors added a commit that referenced this pull request Sep 3, 2013
Significant progress on #6875, enough that I'll open new bugs and turn that into a metabug when this lands.

Description & example in the commit message.
@bors bors closed this Sep 3, 2013
@huonw
Copy link
Member Author

huonw commented Sep 3, 2013

We will want to detect non-leaves too. For example, the name of the std::vec module is not going to change, so that's one of the easiest things in std to mark stable. It's not clear whether there needs to be inheritance of stability levels or not.

I was thinking that

#[stable] 
pub mod vec { 
  pub fn from_elem() { } 
  #[unstable] pub fn from_fn() { } 
}

would mean that from_elem is regarded as stable and from_fn is unstable (I believe this is the way Node works, from my brief glance at their docs). Implementing this would probably require some caches and so on to be efficient.

Once this is fully baked we'll want to turn it on by default so that users know which unstable std and extra API's they are using. How do we do this without making it obnoxious to use their own crates? In other words, we'll put in the effort to mark up std and extra, but we don't want to force users to put #[stable] attrs all over their own work, or get a bunch of false positives by linking to their own crates.

It currently marks experimental and deprecated by default, so I'd think these would be the default in std and extra (especially experimental).

Similarly to above, what happens when using unstable features from within the same crate?

They get marked too; it's not entirely obvious to me how to fix this.

I suspect the term 'unmarked' is a bit unclear here. Maybe also call it unstable but with some additional note that the item isn't annotated.

Agreed.

@thestinger
Copy link
Contributor

If you inherit stability like that, we are going to accidentally stabilize a lot of functions without the necessary discussion.

@huonw
Copy link
Member Author

huonw commented Sep 3, 2013

Hm; it's not necessary to inherit: the only way to actually access those functions will go though the module: i.e. if a crate has a stability annotation then the extern mod foo can be flagged as appropriate, if a mod has one then either the use foo::annotated_mod::bar or ::foo::annotated_mod::bar() can be flagged.

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.

4 participants