Skip to content

Do not report as_conversions for coercions #6428

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

Closed
SOF3 opened this issue Dec 7, 2020 · 8 comments
Closed

Do not report as_conversions for coercions #6428

SOF3 opened this issue Dec 7, 2020 · 8 comments

Comments

@SOF3
Copy link

SOF3 commented Dec 7, 2020

The use of as is discouraged when it is semantically unclear, such as conversion between numeric types. However, when it is used as a coercion, there is no better concise way to indicate the desired semantics.

For example, the following examples are legitimate:

fn identical_position<T>(a: &T, b: &T) -> bool {
    a as *const T == b as *const T
}

fn read(e: Enum) -> &mut dyn Read {
    match e {
        Enum::File(file) => file as &mut dyn Read,
        Enum::Cursor(cursor) => cursor as &mut dyn Read,
    }
}

I understand that as_conversions is set to allow for these reasons. However, i wish to set warn(as_conversions) at the crate-level to prevent future contributors from adding numeric casts to my project. It would be helpful to have a variant of as_conversions only for non-coercing casts, in particular, numeric casts.

@LunaBorowska
Copy link
Contributor

LunaBorowska commented Dec 8, 2020

The first example could be changed to use std::ptr::eq instead. Meanwhile, in the second example as &mut dyn Read shouldn't be necessary (as in, removal of as &mut dyn Read shouldn't make this code not compile).

@SOF3
Copy link
Author

SOF3 commented Dec 9, 2020

For the second example, there are some more complex cases where the compiler cannot infer it to be &mut dyn immediately. I cannot construct such an example by hand, but I think most people have run into this at least once.

@rail-rain
Copy link
Contributor

@SOF3 Could you confirm that you need a single lint that warns every uses of as except coercions, not a bunch of lints that triggers on different situations; already existing as the cast_* family for numeric casts and bonus fn_to_numeric_cast* + char_lit_as_u8. If I understand correctly, as_conversions is for people who want a simple lint to detect every use of as; independent from other as related lints.

@SOF3
Copy link
Author

SOF3 commented Jan 6, 2021

@rail-rain you mean separate lints such as clippy::numeric_as_conversions, clippy::ptr_as_conversions, etc.?

@rail-rain
Copy link
Contributor

@SOF3 Um…I'm afraid not; the bunch of lints for specific cases (e.g. numeric casts that may truncate large values) already exist. You can find these by searching on "cast" here (I couldn't find a way to set that as a query string). I wanted to check these lints do not fit your use case.

@SOF3
Copy link
Author

SOF3 commented Jan 6, 2021

oh yeah, I forgot those exist. should probably have used them instead.

@rail-rain
Copy link
Contributor

In that case, do you still need this issue? Otherwise, could you close the issue please?

@SOF3 SOF3 closed this as completed Jan 7, 2021
@rail-rain
Copy link
Contributor

Thank you!

bors added a commit that referenced this issue Jan 19, 2021
Add a note to `as_conversions`

I have seen a couple of examples where there are some misunderstandings of `as_conversions` ([1](#5890 (comment)), [2](#6116 (comment)) and [3](#6428)). This PR adds the note that explains its purpose and relationship with other `as` related casts. Open question: should I list every related lints for discoverbility, or just suggest how to find these? I chose the former because there's no way to list only and all `as` related lints (e.g. on All the Clippt Lints, 'cast' includes some noises, but `cast_` excludes some) even though I cannot guarantee the list will be updated to include future changes.

---

changelog: Add a note to the document of `as_conversions`
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

No branches or pull requests

3 participants