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

Replace trivial casts with type ascription #23542

Closed
nrc opened this issue Mar 20, 2015 · 10 comments
Closed

Replace trivial casts with type ascription #23542

nrc opened this issue Mar 20, 2015 · 10 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup.

Comments

@nrc
Copy link
Member

nrc commented Mar 20, 2015

There are a bunch of places in the libs and compiler where we use a trivial case where type ascription would be more appropriate. These currently require #![allow(trivial_casts)]. Without that we get a warning (and an error when bootstrapping). When type ascription lands these should be converted to use that. E.g., *self as *const T should become *self: *const T.

@nrc nrc added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Mar 20, 2015
@djmally
Copy link
Contributor

djmally commented Mar 21, 2015

Do you have some examples of where this occurs in the source? I can take a look at this.

@nrc
Copy link
Member Author

nrc commented Mar 21, 2015

Not yet, I haven't landed the patch :-) We haven't landed type ascription either yet, so its not possible to do this yet. Once the two have landed (I'll try to ping this issue when they do), you can grep for FIXME(#23542) to find a bunch of specific places. There are also some crates with #![allow(trivial_casts)] at the crate root which had more problems than I wanted to deal with individually, you can remove that annotation and the compiler will tell you all the problems.

@djmally
Copy link
Contributor

djmally commented Mar 21, 2015

Oh, oops, looks like I didn't read the issue closely enough :). I'll sit tight for now and revisit this when it's ready.

@robinst
Copy link
Contributor

robinst commented Apr 28, 2015

(Blocked by #23416 then, I guess.)

@Manishearth Manishearth removed the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Nov 20, 2015
@Manishearth
Copy link
Member

Not easy, type ascription hasn't landed yet 😄

@nrc
Copy link
Member Author

nrc commented Jan 5, 2016

Type ascription has now landed, so it might be possible to start this (although type ascription does not yet cause coercion, so maybe it can't be just yet).

Here are all the uses of trivial_casts in any case: https://dxr.mozilla.org/rust/search?q=trivial_casts&redirect=true&case=true

@Mark-Simulacrum
Copy link
Member

It feels like based on the following we don't really use trivial casts in the compiler based on the lint allows, though there are a few instances of trivial numeric casts. I've provided a list below of what I could find as of now with a simple grep. I'm uncertain as to whether it's possible to use type ascription already, but I feel like the answer is possibly no.

$ rg -trust trivial_
src/libcore/iter/range.rs
65:            #[allow(trivial_numeric_casts)]
125:            #[allow(trivial_numeric_casts)]

src/librustc_typeck/check/mod.rs
1448:#[allow(trivial_numeric_casts)]

# Tests

src/test/compile-fail/liveness-unused.rs
13:#![allow(dead_code, non_camel_case_types, trivial_numeric_casts)]

src/test/compile-fail/object-safety-by-value-self.rs
15:#![allow(trivial_casts)]

src/test/run-pass/trivial_casts.rs
13:#![allow(trivial_casts, trivial_numeric_casts)]

src/test/compile-fail/trivial_casts.rs
11:// Test the trivial_casts and trivial_numeric_casts lints. For each error we also
14:#![deny(trivial_casts, trivial_numeric_casts)]

@sanmai-NL
Copy link

Type ascription is a nightly-only feature, right?

@Mark-Simulacrum
Copy link
Member

Yes, it is. The tracking issue is #23416.

@Mark-Simulacrum Mark-Simulacrum added C-cleanup Category: PRs that clean code up or issues documenting cleanup. and removed A-refactor labels Jul 22, 2017
@Mark-Simulacrum
Copy link
Member

I'm going to close this as type ascription to me isn't yet ready and won't be for a while; it also doesn't really seem like they kind of cleanup that's super helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup.
Projects
None yet
Development

No branches or pull requests

7 participants