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

View memory layout gives incorrect layout for unions #17091

Open
antonilol opened this issue Apr 17, 2024 · 3 comments
Open

View memory layout gives incorrect layout for unions #17091

antonilol opened this issue Apr 17, 2024 · 3 comments
Labels
C-bug Category: bug

Comments

@antonilol
Copy link

rust-analyzer version: rust-analyzer version: 0.4.1830-standalone

rustc version: rustc 1.77.0 (aedd173a2 2024-03-17)

editor or extension: Code OSS (on arch linux, installed with pacman, code 1.87.2-1)

relevant settings: None

code snippet to reproduce:

union Union {
    a: u16,
    b: u32,
}

result when hitting Ctrl+Shift+P and going to 'view memory layout':
image

when hovering on the b: u32 field, the tooltip reports an offset of 0, although it is not placed at an offset of 0 in here:
image

this is a cool feature, but it just needs some more polishing:

for example, it knows PhantomData<T> always has zero size, although in the tooltip it will say A<{unknown}>, but [T; 0] also has zero size and for that it says 'Not Available'

struct Struct<T> {
    a: usize,
    b: PhantomData<T>,
}

it does not account for fat pointers:

struct Struct<'a, T: ?Sized> {
    a: usize,
    b: &'a T,
}

this will report the &T being 8 bytes wide (&{unknown} here too). this is only correct if T: Sized

and for enums just size and align is displayed, nothing is done with the members

@antonilol antonilol added the C-bug Category: bug label Apr 17, 2024
@roife
Copy link
Contributor

roife commented Apr 18, 2024

let mut fields = ty
.fields(db)
.into_iter()
.map(|(f, ty)| (FieldOrTupleIdx::Field(f), ty))
.chain(
ty.tuple_fields(db)
.into_iter()
.enumerate()
.map(|(i, ty)| (FieldOrTupleIdx::TupleIdx(i), ty)),
)
.collect::<Vec<_>>();
if fields.is_empty() {
return;
}

Here we're simply using .fields() to fetch all the fields, which works for both struct and union (but does not work for enum). Do we need to add viewing-memory-layout for unions? Or simply add a check to bail out if it's a union. 👀

@Veykril
Copy link
Member

Veykril commented Apr 18, 2024

Yes we need to check what kind of ADT we are dealing with here, ideally we'd render things differently for union showing that they overlap.

Tbf, this feature needs a design overhaul, it is very hard to parse imo :) Iirc Visual Studio (not Code) has a very nice looking one, and there is an extension for vscode that has a really nice one for C++ that I can't recall the name of (might've been builtin into the clang extension)

@roife
Copy link
Contributor

roife commented Apr 18, 2024

there is an extension for vscode

Might be https://marketplace.visualstudio.com/items?itemName=RamonViladomat.StructLayout. Really cool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

3 participants