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

Lint on pointer comparisons for non-thin pointers. #117717

Closed
scottmcm opened this issue Nov 8, 2023 · 0 comments · Fixed by #117758
Closed

Lint on pointer comparisons for non-thin pointers. #117717

scottmcm opened this issue Nov 8, 2023 · 0 comments · Fixed by #117758
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. E-help-wanted Call for participation: Help is requested to fix this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

scottmcm commented Nov 8, 2023

Per the 2023-11-08 lang triage meeting, lang would like to see a lint like the one in the second bullet from #106447 (comment) :

Deprecate comparisons (Eq/Ord) on dyn trait pointers by adding a lint which encourages people to case the pointer to *mut u8 first. This makes the intent of such comparisons much clearer.

The lint should not warn on comparison of thin pointers.

When non-thin pointers are compared (whether with == or != or PartialEq methods directly), the lint should warn, suggesting that you do one of the following instead:
@scottmcm scottmcm added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. E-help-wanted Call for participation: Help is requested to fix this issue. labels Nov 8, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 8, 2023
@scottmcm scottmcm mentioned this issue Nov 8, 2023
Closed
@saethlin saethlin added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 8, 2023
@Urgau Urgau mentioned this issue Nov 9, 2023
Merged
@bors bors closed this as completed in 8a37655 Dec 11, 2023
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Dec 12, 2023
@bors
Auto merge of #117758 - Urgau:lint_pointer_trait_comparisons, r=david…
2a8296d 
…twco

Add lint against ambiguous wide pointer comparisons

This PR is the resolution of rust-lang/rust#106447 decided in rust-lang/rust#117717 by T-lang.

## `ambiguous_wide_pointer_comparisons`

*warn-by-default*

The `ambiguous_wide_pointer_comparisons` lint checks comparison of `*const/*mut ?Sized` as the operands.

### Example

```rust
let ab = (A, B);
let a = &ab.0 as *const dyn T;
let b = &ab.1 as *const dyn T;

let _ = a == b;
```

### Explanation

The comparison includes metadata which may not be expected.

-------

This PR also drops `clippy::vtable_address_comparisons` which is superseded by this one.

~~One thing: is the current naming right? `invalid` seems a bit too much.~~

Fixes rust-lang/rust#117717
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this issue Dec 13, 2023
@bors
Auto merge of #117758 - Urgau:lint_pointer_trait_comparisons, r=david…
b5c0dd6 
…twco

Add lint against ambiguous wide pointer comparisons

This PR is the resolution of rust-lang/rust#106447 decided in rust-lang/rust#117717 by T-lang.

## `ambiguous_wide_pointer_comparisons`

*warn-by-default*

The `ambiguous_wide_pointer_comparisons` lint checks comparison of `*const/*mut ?Sized` as the operands.

### Example

```rust
let ab = (A, B);
let a = &ab.0 as *const dyn T;
let b = &ab.1 as *const dyn T;

let _ = a == b;
```

### Explanation

The comparison includes metadata which may not be expected.

-------

This PR also drops `clippy::vtable_address_comparisons` which is superseded by this one.

~~One thing: is the current naming right? `invalid` seems a bit too much.~~

Fixes rust-lang/rust#117717
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Dec 16, 2023
@bors
Auto merge of #117758 - Urgau:lint_pointer_trait_comparisons, r=david…
c8213a4 
…twco

Add lint against ambiguous wide pointer comparisons

This PR is the resolution of rust-lang/rust#106447 decided in rust-lang/rust#117717 by T-lang.

## `ambiguous_wide_pointer_comparisons`

*warn-by-default*

The `ambiguous_wide_pointer_comparisons` lint checks comparison of `*const/*mut ?Sized` as the operands.

### Example

```rust
let ab = (A, B);
let a = &ab.0 as *const dyn T;
let b = &ab.1 as *const dyn T;

let _ = a == b;
```

### Explanation

The comparison includes metadata which may not be expected.

-------

This PR also drops `clippy::vtable_address_comparisons` which is superseded by this one.

~~One thing: is the current naming right? `invalid` seems a bit too much.~~

Fixes rust-lang/rust#117717
liketechnik added a commit to cmjava2023/vm that referenced this issue Dec 19, 2023
@liketechnik
fix(opcode/Ifacmp): compare objects by data pointer only
37a43ea 
We store class instances as Rc<dyn ClassInstanc> values.
The pointers to the inner values consist of
1. a pointer to the vtable of the trait
2. a pointer to the data itself
Due to rust internals, the vtable pointer can be different
for Rc<dyn ClassInstance> values that point to the same *data*,
effectively making any comparison involing the vtable pointer moot.
This uses a function rust just introduced to compare such pointers.
(cf. rust-lang/rust#106447,
rust-lang/rust#117717,
rust-lang/rust#116325)
@RalfJung RalfJung mentioned this issue Jan 9, 2024
Closed
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
@bors
Auto merge of #117758 - Urgau:lint_pointer_trait_comparisons, r=david…
df0fd1f 
…twco

Add lint against ambiguous wide pointer comparisons

This PR is the resolution of rust-lang/rust#106447 decided in rust-lang/rust#117717 by T-lang.

## `ambiguous_wide_pointer_comparisons`

*warn-by-default*

The `ambiguous_wide_pointer_comparisons` lint checks comparison of `*const/*mut ?Sized` as the operands.

### Example

```rust
let ab = (A, B);
let a = &ab.0 as *const dyn T;
let b = &ab.1 as *const dyn T;

let _ = a == b;
```

### Explanation

The comparison includes metadata which may not be expected.

-------

This PR also drops `clippy::vtable_address_comparisons` which is superseded by this one.

~~One thing: is the current naming right? `invalid` seems a bit too much.~~

Fixes rust-lang/rust#117717
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
@bors
Auto merge of #117758 - Urgau:lint_pointer_trait_comparisons, r=david…
9290748 
…twco

Add lint against ambiguous wide pointer comparisons

This PR is the resolution of rust-lang/rust#106447 decided in rust-lang/rust#117717 by T-lang.

## `ambiguous_wide_pointer_comparisons`

*warn-by-default*

The `ambiguous_wide_pointer_comparisons` lint checks comparison of `*const/*mut ?Sized` as the operands.

### Example

```rust
let ab = (A, B);
let a = &ab.0 as *const dyn T;
let b = &ab.1 as *const dyn T;

let _ = a == b;
```

### Explanation

The comparison includes metadata which may not be expected.

-------

This PR also drops `clippy::vtable_address_comparisons` which is superseded by this one.

~~One thing: is the current naming right? `invalid` seems a bit too much.~~

Fixes rust-lang/rust#117717
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: Lints (warnings about flaws in source code) such as unused_mut. E-help-wanted Call for participation: Help is requested to fix this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add lint against ambiguous wide pointer comparisons Urgau/rust
3 participants
@saethlin @scottmcm @rustbot