Skip to content

Change concurrency primitives to standard naming conventions #7978

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

Merged
merged 2 commits into from
Jul 28, 2013

Conversation

mstewartgallus
Copy link
Contributor

To be more specific:

UPPERCASETYPE was changed to UppercaseType
type_new was changed to Type::new
type_function(value) was changed to value.method()

@bblum
Copy link
Contributor

bblum commented Jul 22, 2013

Apart from my minor nits, this looks great. Thanks for doing this.

@mstewartgallus
Copy link
Contributor Author

@bblum c_void is declared as pub enum c_void {} so the codegen issues are the same. I would be fine with having the runtime type as lower case though.

Actually, this doesn't really matter to me. Also, the helper functions don't show up in the documentation.

@bblum
Copy link
Contributor

bblum commented Jul 22, 2013

also please @bblum me on arc-related pull requests?

@bblum
Copy link
Contributor

bblum commented Jul 22, 2013

Oh, didn't realize c_void was already an empty enum. In that case, then, c_void is definitely the right thing to use for FFI-C-pointer types.

@thestinger
Copy link
Contributor

@bblum: if they're opaque pointer types they should be their own uninhabited enum declarations, to avoid needlessly using weak typing

@mstewartgallus
Copy link
Contributor Author

@bblum You're right I should have added a CC @bblum to the summary of the pull request. I'll remember to do so in the future when working on arc related stuff.

@thestinger I agree but that can wait until another pull request. I plan on doing some more work on this stuff. Also, isn't the runtime being rewritten in Rust anyways (or is this specific part going to still be in C++?)

@mstewartgallus
Copy link
Contributor Author

Had to rebase this because of 6174f9a .

@bblum
Copy link
Contributor

bblum commented Jul 23, 2013

@thestinger Not sure why weak typing is bad. libc::c_void is a better name for exactly what it's used for, whereas a fresh empty enum declaration could leave a reader wondering exactly why it's unhabitable.

(good heavens, just look at the time!)

@mstewartgallus
Copy link
Contributor Author

Can I get an r+, or criticism on why one should not be forthcoming? I don't want this pull request to just sit here, and bitrot.

@bblum
Copy link
Contributor

bblum commented Jul 27, 2013

Criticism: I didn't know you had rebased again and needed another one. Apparently github does not send emails about such things.

@mstewartgallus
Copy link
Contributor Author

Yeah, there seem to be a lot of problems with rebasing, and GitHub. I'll just add another commit next time.

@bblum
Copy link
Contributor

bblum commented Jul 28, 2013

I'd prefer to leave a comment; I don't like adding unnecessary extra commits, myself.

@mstewartgallus
Copy link
Contributor Author

Okay. I squashed it into the previous commit.

@bblum
Copy link
Contributor

bblum commented Jul 28, 2013

Can't merge...

To be more specific:

`UPPERCASETYPE` was changed to `UppercaseType`
`type_new` was changed to `Type::new`
`type_function(value)` was changed to `value.method()`
@mstewartgallus
Copy link
Contributor Author

Rebased.

bors added a commit that referenced this pull request Jul 28, 2013
To be more specific:

`UPPERCASETYPE` was changed to `UppercaseType`
`type_new` was changed to `Type::new`
`type_function(value)` was changed to `value.method()`
@bors bors closed this Jul 28, 2013
@bors bors merged commit 39b3a05 into rust-lang:master Jul 28, 2013
@mstewartgallus mstewartgallus deleted the rename_arc branch July 28, 2013 15:44
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 17, 2021
Add `unnecessary_to_owned` lint

This PR adds a lint to check for unnecessary calls to `ToOwned::to_owned` and other similar functions (e.g., `Cow::into_owned`, `ToString::to_string`, etc.).

The lint checks for expressions of the form `&receiver.to_owned_like()` used in a position requiring type `&T` where one of the following is true:
* `receiver`'s type is `T` exactly
* `receiver`'s type implements `Deref<Target = T>`
* `receiver`'s type implements `AsRef<T>`

The lint additionally checks for expressions of the form `receiver.to_owned_like()` used as arguments of type `impl AsRef<T>`.

It would be nice if the lint could also check for expressions used as arguments to functions like the following:
```
fn foo<T: AsRef<str>>(x: T) { ... }
```
However, I couldn't figure out how to determine whether a function input type was instantiated from a parameter with a trait bound.

If someone could offer me some guidance, I would be happy to add such functionality.

Closes rust-lang#7933

changelog: Add [`unnecessary_to_owned`] lint
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