Skip to content

Remove old #[allow(trivial_casts)] annotations #31492

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 1 commit into from
Feb 9, 2016

Conversation

alexcrichton
Copy link
Member

These were added a long time ago but we long since switched the lint back to
allow-by-default, so these annotations shouldn't be necessary.

These were added a long time ago but we long since switched the lint back to
allow-by-default, so these annotations shouldn't be necessary.
@rust-highfive
Copy link
Contributor

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

r? @nrc

@rust-highfive rust-highfive assigned nrc and unassigned brson Feb 8, 2016
@nrc
Copy link
Member

nrc commented Feb 8, 2016

Can you leave the FIXMEs in please? I believe they are still valid. r+ with that.

@alexcrichton
Copy link
Member Author

Hm, would the FIXME still make sense without a #![allow] there? It seems odd to be annotating as casts with FIXMEs when it's the only way to do so no?

@nrc
Copy link
Member

nrc commented Feb 9, 2016

The point is once we support coercion in type ascription, then we replace the as with type ascription, i.e., it is the only way now, but in the future it won't be.

@alexcrichton
Copy link
Member Author

Right but this means the code looks like:

     fn fmt(&self, f: &mut Formatter) -> Result {
        // FIXME(#23542) Replace with type ascription.
        Pointer::fmt(&(*self as *const T), f)
    } 

which seems odd because we're not necessarily stylistically attempting to annotate all as casts with a FIXME (there are likely quite a few throughout core/std). Once type ascription is landed we can in theory turn back on the lint to catch cases like this, right?

@nrc
Copy link
Member

nrc commented Feb 9, 2016

Yeah, I guess so.

@nrc
Copy link
Member

nrc commented Feb 9, 2016

@bors: r+

@bors
Copy link
Collaborator

bors commented Feb 9, 2016

📌 Commit 696a1da has been approved by nrc

bors added a commit that referenced this pull request Feb 9, 2016
These were added a long time ago but we long since switched the lint back to
allow-by-default, so these annotations shouldn't be necessary.
@bors
Copy link
Collaborator

bors commented Feb 9, 2016

⌛ Testing commit 696a1da with merge 0d410b8...

@bors bors merged commit 696a1da into rust-lang:master Feb 9, 2016
@alexcrichton alexcrichton deleted the remove-allow-trivial-casts branch February 12, 2016 20:57
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.

5 participants