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.

Sign up for GitHub

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

Jump to bottom

RefCell<T> where T: Copy and T in current crate #1

Open
pythonesque opened this issue Nov 19, 2014 · 2 comments · May be fixed by #13388
Open

RefCell<T> where T: Copy and T in current crate #1

pythonesque opened this issue Nov 19, 2014 · 2 comments · May be fixed by #13388
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-middle Type: Probably requires verifiying types

Comments

@pythonesque
Copy link
Contributor

In this situation you can always replace it with finer-grained Cells, you'd only turn it off if you have some specific reason not to do that. In my experience this is usually a bug.
@lambda-fairy
Copy link

What's the reason for checking that T is in the current crate?

@pythonesque
Copy link
Contributor Author

@lfairy If it's not in the current crate, it's possible that you can't refactor a type that is Copy, but large, by making smaller elements Copy instead. If you can't do that, it's possible (albeit unlikely) that the overhead of copying the entire type would make RefCell attractive. Maybe this lint could be refined so that it warns unless the type is "large" and not in the current crate, but I'm not sure how much of that information is actually available at lint time.

@Manishearth Manishearth added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-middle Type: Probably requires verifiying types A-lint Area: New lints labels Aug 11, 2015
@oli-obk oli-obk mentioned this issue May 12, 2016
Closed
@p-kraszewski p-kraszewski mentioned this issue Jan 26, 2017
Closed
oli-obk pushed a commit that referenced this issue Apr 24, 2018
@Xandkeeper
Merge pull request #1 from humanenginuity/fix-strictly-aligned-types-…
efdd00d 
…cast-messages

Corrected messaging to warn against less- to more-strictly aligned
@clarfonthey clarfonthey mentioned this issue Jun 16, 2018
Open
bors pushed a commit that referenced this issue Feb 1, 2019
@phansch
Travis: Don't run integration tests on every PR commit
4e39e65 
This does not save Clippy any time but it makes sure that the concurrent
build limit is not reached as quickly for the `rust-lang` Travis
account.

I can't create a permalink to the discussion somehow, so here's an
excerpt from the Infra channel:

```
[11:57 PM] pietroalbini: there is a clippy build (20 jobs) and a packed_simd one (42 builders)  and a rustc one which isn't scheduling atm
[11:58 PM] pietroalbini: I don't think there is a way to prioritize rustc builds in the queue on travis, right?
[12:22 AM] alexcrichton: pietro: I don't think so no
[12:22 AM] alexcrichton: If it's a problem we should cull builds on other projects
[12:22 AM] alexcrichton: The rust repo is the #1 priority
```

Since the integration tests are rarely failing these days, I think it's
fine to not run them on every commit.

If needed, it's also still possible to do a `try` build with `@bors try`.
bors added a commit that referenced this issue Feb 1, 2019
@bors
Auto merge of #3731 - phansch:travis, r=<try>
cadf7d7 
Travis: Don't run integration tests on every PR commit

This does not save Clippy any time but it makes sure that the concurrent
build limit is not reached as quickly for the `rust-lang` Travis
account.

I can't create a permalink to the discussion somehow, so here's an
excerpt from the Infra channel:

```
[11:57 PM] pietroalbini: there is a clippy build (20 jobs) and a packed_simd one (42 builders)  and a rustc one which isn't scheduling atm
[11:58 PM] pietroalbini: I don't think there is a way to prioritize rustc builds in the queue on travis, right?
[12:22 AM] alexcrichton: pietro: I don't think so no
[12:22 AM] alexcrichton: If it's a problem we should cull builds on other projects
[12:22 AM] alexcrichton: The rust repo is the #1 priority
```

Since the integration tests are rarely failing these days, I think it's
fine to not run them on every commit.

If needed, it's also still possible to do a `try` build with `@bors try`.
bors added a commit that referenced this issue Feb 2, 2019
@bors
Auto merge of #3731 - phansch:travis, r=phansch
9f88641 
Travis: Don't run integration tests on every PR commit

This does not save Clippy any time but it makes sure that the concurrent
build limit is not reached as quickly for the `rust-lang` Travis
account.

I can't create a permalink to the discussion somehow, so here's an
excerpt from the Infra channel:

```
[11:57 PM] pietroalbini: there is a clippy build (20 jobs) and a packed_simd one (42 builders)  and a rustc one which isn't scheduling atm
[11:58 PM] pietroalbini: I don't think there is a way to prioritize rustc builds in the queue on travis, right?
[12:22 AM] alexcrichton: pietro: I don't think so no
[12:22 AM] alexcrichton: If it's a problem we should cull builds on other projects
[12:22 AM] alexcrichton: The rust repo is the #1 priority
```

Since the integration tests are rarely failing these days, I think it's
fine to not run them on every commit.

If needed, it's also still possible to do a complete test run with `@bors try`.
g-bartoszek pushed a commit to g-bartoszek/rust-clippy that referenced this issue Feb 7, 2019
@phansch
Travis: Don't run integration tests on every PR commit
663f369 
This does not save Clippy any time but it makes sure that the concurrent
build limit is not reached as quickly for the `rust-lang` Travis
account.

I can't create a permalink to the discussion somehow, so here's an
excerpt from the Infra channel:

```
[11:57 PM] pietroalbini: there is a clippy build (20 jobs) and a packed_simd one (42 builders)  and a rustc one which isn't scheduling atm
[11:58 PM] pietroalbini: I don't think there is a way to prioritize rustc builds in the queue on travis, right?
[12:22 AM] alexcrichton: pietro: I don't think so no
[12:22 AM] alexcrichton: If it's a problem we should cull builds on other projects
[12:22 AM] alexcrichton: The rust repo is the rust-lang#1 priority
```

Since the integration tests are rarely failing these days, I think it's
fine to not run them on every commit.

If needed, it's also still possible to do a `try` build with `@bors try`.
@matthiaskrgr matthiaskrgr mentioned this issue Oct 27, 2020
Closed
@d1mmu-dev d1mmu-dev mentioned this issue Oct 30, 2020
Closed
@royster57 royster57 mentioned this issue Nov 2, 2020
Closed
@ebroto ebroto mentioned this issue Dec 15, 2020
Closed
@Fishrock123 Fishrock123 mentioned this issue Mar 17, 2021
Closed
xFrednet referenced this issue in xFrednet/rust-clippy May 15, 2022
@PrestonFrom
Lint for significant drops who may have surprising lifetimes #1
41c7e4d 
author Preston From <[email protected]> 1645164142 -0600
committer Preston From <[email protected]> 1650005351 -0600
botahamec pushed a commit to botahamec/rust-clippy that referenced this issue May 21, 2022
@PrestonFrom @botahamec
Lint for significant drops who may have surprising lifetimes rust-lang#1
25176f3 


author Preston From <[email protected]> 1645164142 -0600
committer Preston From <[email protected]> 1650005351 -0600
@bobi6666 bobi6666 mentioned this issue May 25, 2022
Closed
@Jarcho Jarcho mentioned this issue Sep 25, 2022
Merged
kraktus pushed a commit to kraktus/rust-clippy that referenced this issue Oct 2, 2022
@blyxyas
Merge pull request rust-lang#1 from kraktus/pr-9506
5cbd8df 
Suggestions for rust-lang#9506
@rick-kilgore rick-kilgore mentioned this issue Oct 30, 2022
Closed
@kwannoel kwannoel mentioned this issue Apr 10, 2023
Closed
@LeonGGX LeonGGX mentioned this issue Jun 8, 2023
Open
J-ZhengLi pushed a commit to J-ZhengLi/rust-clippy that referenced this issue Jun 26, 2023
@surechen
Merge pull request rust-lang#1 from J-ZhengLi/dev_stable
38ab3a3 
add new lints: [`mem_unsafe_functions`], [`unsafe_block_in_proc_macro`] and [`implicit_abi`]
surechen added a commit to surechen/rust-clippy that referenced this issue Jun 26, 2023
@surechen
Squashed commit of the following:
4f6422c 
commit 3a413b7
Merge: d5a1eff 4d8af6f
Author: surechen <[email protected]>
Date:   Mon Jun 26 11:32:14 2023 +0800

    Merge pull request rust-lang#5 from mojave2/lint_loop_exit

    new_lint loop_without_break_or_return

commit 4d8af6f
Author: mojave2 <[email protected]>
Date:   Sun Jun 25 19:21:11 2023 +0800

    new_line loop_without_break_or_return

    Signed-off-by: mojave2 <[email protected]>

commit d5a1eff
Merge: af6c9d4 9cd4b13
Author: surechen <[email protected]>
Date:   Mon Jun 26 11:23:44 2023 +0800

    Merge pull request rust-lang#2 from mojave2/dev_stable

    new_lint non_reentrant_functions

commit 9cd4b13
Author: mojave2 <[email protected]>
Date:   Wed Jun 21 17:44:12 2023 +0800

    update_lint non_reentrant_functions

    Signed-off-by: mojave2 <[email protected]>

commit c900d4a
Author: mojave2 <[email protected]>
Date:   Mon Jun 19 18:37:16 2023 +0800

    update_lint non_reentrant_functions

    Signed-off-by: mojave2 <[email protected]>

commit 9845b3a
Author: mojave2 <[email protected]>
Date:   Sat Jun 17 16:32:10 2023 +0800

    improve implementation by dogfood

    Signed-off-by: mojave2 <[email protected]>

commit a3a1415
Author: mojave2 <[email protected]>
Date:   Sat Jun 17 15:29:11 2023 +0800

    doc-comment non_reentrant_functions

    Signed-off-by: mojave2 <[email protected]>

commit eb80bb0
Author: mojave2 <[email protected]>
Date:   Sat Jun 17 15:14:34 2023 +0800

    new_lint non_reentrant_functions

    Signed-off-by: mojave2 <[email protected]>

commit af6c9d4
Merge: 38ab3a3 33ddd19
Author: surechen <[email protected]>
Date:   Mon Jun 26 10:54:08 2023 +0800

    Merge pull request rust-lang#7 from J-ZhengLi/dev_stable

    fix test fail after dev fmt of [`implicit_abi`]

commit 33ddd19
Author: J-ZhengLi <[email protected]>
Date:   Mon Jun 26 10:15:26 2023 +0800

    fix test fail after dev fmt of [`implicit_abi`]

    Signed-off-by: J-ZhengLi <[email protected]>

commit 38ab3a3
Merge: 83e42a2 5ce5313
Author: surechen <[email protected]>
Date:   Mon Jun 26 09:47:22 2023 +0800

    Merge pull request rust-lang#1 from J-ZhengLi/dev_stable

    add new lints: [`mem_unsafe_functions`], [`unsafe_block_in_proc_macro`] and [`implicit_abi`]

commit 5ce5313
Author: J-ZhengLi <[email protected]>
Date:   Wed Jun 21 11:24:29 2023 +0800

    add new lint [`implicit_abi`]
    that detects external block without explicit label "C"

    Signed-off-by: J-ZhengLi <[email protected]>

commit cc5ab4a
Author: J-ZhengLi <[email protected]>
Date:   Tue Jun 20 12:44:02 2023 +0800

    add new lint [`unsafe_block_in_proc_macro`]
    that detects unsafe block in `quote!`

    Signed-off-by: J-ZhengLi <[email protected]>

commit c091b77
Author: J-ZhengLi <[email protected]>
Date:   Sat Jun 17 15:59:56 2023 +0800

    add configuration for lint [`mem_unsafe_functions`];
    finish implement lint [`mem_unsafe_functions`];
    add simple description for unimplemented lints;

    Signed-off-by: J-ZhengLi <[email protected]>

commit a50f308
Author: J-ZhengLi <[email protected]>
Date:   Fri Jun 16 17:45:56 2023 +0800

    init following new lints:

    [`passing_string_to_c_functions`]
    [`untrusted_lib_loading`]
    [`falliable_memory_allocation`]

    Signed-off-by: J-ZhengLi <[email protected]>

commit da3cc15
Author: J-ZhengLi <[email protected]>
Date:   Fri Jun 16 14:11:59 2023 +0800

    add `guidelines` type and barebone for [`mem_unsafe_functions`] lint

    Signed-off-by: J-ZhengLi <[email protected]>

# Conflicts:
#	clippy_lints/src/lib.rs
@leonardkulms leonardkulms mentioned this issue Jul 14, 2023
Open
bors added a commit that referenced this issue Aug 23, 2023
@bors
Auto merge of #11379 - popzxc:fix-tuple-array-conversions, r=xFrednet
edfee16 
Fix tuple_array_conversions lint on nightly

```
changelog: ICE: [`tuple_array_conversions`]: Don't expect array length to always be usize
```

tl;dr: changed [`Const::eval_target_usize`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_middle/src/ty/consts.rs#L359) to [`Consts::try_eval_target_usize`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_middle/src/ty/consts.rs#L327) to get rid of ICE.

I have encountered a problem with clippy: it caught ICE when working with a codebase that uses a lot of nightly features.
Here's a (stripped) ICE info:

```
error: internal compiler error: /rustc/5c6a7e71cd66705c31c9af94077901a220f0870c/compiler/rustc_middle/src/ty/consts.rs:361:32: expected usize, got Const { ty: usize, kind: N/#1 }

thread 'rustc' panicked at /rustc/5c6a7e71cd66705c31c9af94077901a220f0870c/compiler/rustc_errors/src/lib.rs:1635:9:
Box<dyn Any>
stack backtrace:
...
  16:        0x110b9c590 - rustc_middle[449edf845976488d]::util::bug::bug_fmt
  17:        0x102f76ae0 - clippy_lints[71754038dd04c2d2]::tuple_array_conversions::all_bindings_are_for_conv
...
```

I don't really know what's going on low-level-wise, but seems like this lin assumed that the length of the array can always be treated as `usize`, and *I assume* this doesn't play well with `feat(generic_const_exprs)`.

I wasn't able to build a minimal reproducible example, but locally this fix does resolve the issue.
@mkravchik mkravchik mentioned this issue Apr 17, 2024
Closed
@lukaslueg lukaslueg mentioned this issue Aug 16, 2024
Open
@GnomedDev GnomedDev linked a pull request Sep 12, 2024 that will close this issue
Open
@GnomedDev GnomedDev mentioned this issue Oct 16, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-middle Type: Probably requires verifiying types
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement a lint for RefCell<impl Copy> GnomedDev/rust-clippy
3 participants
@pythonesque @lambda-fairy @Manishearth