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

Detect stability attributes on methods. #10990

Merged
merged 1 commit into from
Dec 17, 2013
Merged

Conversation

ktt3ja
Copy link
Contributor

@ktt3ja ktt3ja commented Dec 16, 2013

If it's a trait method, this checks the stability attribute of the
method inside the trait definition. Otherwise, it checks the method
implementation itself.

Close #8961.

Some(trait_did) => {
let trait_methods = ty::trait_methods(tcx, trait_did);
match trait_methods.iter().position(
|m| { m.ident.name == name }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is completely correct. I essentially copied what's done here: https://github.com/mozilla/rust/blob/master/src/librustc/middle/typeck/check/method.rs#L500-L503

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this need to copy that logic exactly? i.e. also ignore trait methods without a self (e.g. both bar and baz in trait Foo { fn bar(n: uint) -> Self; fn baz() -> uint; }) (that's what the != sty_static is checking).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't understand why it ignores trait methods without self. Is it possible for the name to overlap still? And can't you implement a static trait method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I guess the name can't overlap. shrugs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I'll leave it as is. If some bug comes up, at least I'll know what it's for. :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be

trait_methods.iter()
    .position(|m| m.ident.name == name)
    .map(|idx| ty::trait_method(tcx, trait_did, idx).def_id)

(Copying my commit comment here so it's preserved post-rebase/update.)

@ktt3ja
Copy link
Contributor Author

ktt3ja commented Dec 16, 2013

r? @huonw

@ktt3ja
Copy link
Contributor Author

ktt3ja commented Dec 16, 2013

r? @huonw. I updated the comment to ty::trait_method_of_method and added test_method_param and test_method_object to the cross_crate module.

// of the method inside trait definition.
// Otherwise, use the current def_id (which refers
// to the method inside impl).
match ty::trait_method_of_method(cx.tcx, def_id) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this could be ty::trait_method_of_method(cx.tcx, def_id).unwrap_or(def_id).

(Copying my commit comment here so it's preserved post-rebase/update.)

@huonw
Copy link
Member

huonw commented Dec 16, 2013

r=me after the two option-matching style nits are fixed. (Thanks so much for this!)

@brson
Copy link
Contributor

brson commented Dec 16, 2013

Very glad to see work on this!

If it's a trait method, this checks the stability attribute of the
method inside the trait definition. Otherwise, it checks the method
implementation itself.
@eddyb
Copy link
Member

eddyb commented Dec 17, 2013

@brson you and me both :). I have an experiment for which this will be perfect.

bors added a commit that referenced this pull request Dec 17, 2013
If it's a trait method, this checks the stability attribute of the
method inside the trait definition. Otherwise, it checks the method
implementation itself.

Close #8961.
@bors bors closed this Dec 17, 2013
@bors bors merged commit 4f95dce into rust-lang:master Dec 17, 2013
@ktt3ja ktt3ja deleted the method-stability branch December 17, 2013 18:24
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 30, 2023
…dnet

[`single_match`]: don't lint if block contains comments

Fixes rust-lang#8634

It now ignores matches with a comment in the "else" arm

changelog: [`single_match`]: don't lint if block contains comments
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.

Detect stability attributes on methods
5 participants