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

Add attributes for marking anonymous and unnamed structs #99

Closed
DefaultRyan opened this issue Jan 11, 2021 · 10 comments
Closed

Add attributes for marking anonymous and unnamed structs #99

DefaultRyan opened this issue Jan 11, 2021 · 10 comments
Labels
usability Touch-up to improve the user experience for a language projection

Comments

@DefaultRyan
Copy link
Member

No description provided.

@AArnott AArnott added the usability Touch-up to improve the user experience for a language projection label Jan 14, 2021
@timsneath
Copy link
Contributor

Just want to flag this one to try and raise its priority, and add some additional context as to why this is important.

Contrast these two structs:

typedef struct _BLUETOOTH_ADDRESS {
    union {
        BTH_ADDR ullLong;       //  easier to compare again BLUETOOTH_NULL_ADDRESS
        BYTE    rgBytes[ 6 ];   //  easier to format when broken out
    };

} BLUETOOTH_ADDRESS_STRUCT;

typedef struct _CHAR_INFO {
    union {
        WCHAR UnicodeChar;
        CHAR   AsciiChar;
    } Char;
    WORD Attributes;
} CHAR_INFO, *PCHAR_INFO;

In metadata these become:

.class public sequential ansi sealed beforefieldinit Windows.Win32.Devices.Bluetooth.BLUETOOTH_ADDRESS
	extends [netstandard]System.ValueType
{
	.class nested public explicit ansi sealed beforefieldinit _Anonymous_e__Union
		extends [netstandard]System.ValueType
	{
		.field [0] public uint64 ullLong
		.field [0] public uint8[0...5] rgBytes
	}

	.field public valuetype [Windows.Win32.winmd]Windows.Win32.Devices.Bluetooth.BLUETOOTH_ADDRESS/_Anonymous_e__Union Anonymous
}

.class public sequential ansi sealed beforefieldinit Windows.Win32.System.Console.CHAR_INFO
	extends [netstandard]System.ValueType
{
	.class nested public explicit ansi sealed beforefieldinit _Char_e__Union
		extends [netstandard]System.ValueType
	{
		.field [0] public char UnicodeChar
		.field [0] public valuetype [Windows.Win32.winmd]Windows.Win32.Foundation.CHAR AsciiChar

	} 

	.field public valuetype [Windows.Win32.winmd]Windows.Win32.System.Console.CHAR_INFO/_Char_e__Union Char
	.field public uint16 Attributes

}

There is no metadata difference between the two, yet they have distinct signatures. In C code:

    CHAR_INFO c;
    BLUETOOTH_ADDRESS b;
    c.Char.AsciiChar = 0x41; // A
    b.ullLong = 0;

For a projection, the lack of distinction between these two different signatures means that I cannot accurately model the underlying API. If all anonymous unions and structs had a consistent name, I could perhaps interpolate whether they were anonymous, but that's not the case either.

@sotteson1
Copy link
Contributor

sotteson1 commented Mar 18, 2022

The difference is that CHAR_INFO named the union (Char), so the scraper uses that in the name. (It has to have a name in the metadata.) In BLUETOOTH_ADDRESS_STRUCT the union is unnamed, so the scraper called it Anonymous. So the difference is that in one struct the union is named and the other it isn't.

C chooses to expose the unnamed union as if they were members of the original struct. A projection could do that too if it wanted to.

@timsneath
Copy link
Contributor

Right, I get that.

But that very significant difference in the C API isn't carried across into the metadata. I can model both approaches in my projection, but for a given struct I don't know whether the nested union is named or unnamed.

@sotteson1
Copy link
Contributor

_Anonymous_e__Union

Couldn't you look for the pattern: _(.+)_e__Union

Then look at the captured name. If it's Anonymous, then it's an unnamed union. Otherwise you use the name you captured as its name.

@kennykerr
Copy link
Contributor

kennykerr commented Mar 18, 2022

The names of nested types are generally so unwieldy I just scrap the names of any nested structs and number them uniquely based on the enclosing type. For example, here's what CHAR_INFO and BLUETOOTH_ADDRESS look like in Rust. It's not pretty but it's far more predictable and coherent for the developer. Also, Rust lacks supports for nested types so I have to mangle the names.

@timsneath
Copy link
Contributor

timsneath commented Mar 19, 2022

@sotteson1 thanks! That's helpful, if that's dependable and durable even with multiple layers of nested anonymous types. But what you're describing is metadata, isn't it? Shouldn't it be encapsulated in the database, rather than needing to be interpolated?

@kennykerr, interesting. For Dart, I'm able to use extensions to lift nested types into the parent, but I want to hide the nested anonymous property if it's not necessary.

@mikebattista
Copy link
Contributor

What is the conclusion here? Is this something projections should handle themselves?

@timsneath
Copy link
Contributor

In an ideal world, I think you might use the heuristic proposed by @sotteson1 and apply an 'anonymous' attribute to those classes that are anonymous. Language projections could individually apply this heuristic independently of the .winmd file (if they know about it), but it is metadata, it is present in the headers, and it is semantically relevant to language projections.

@mikebattista
Copy link
Contributor

I don't see that recommendation. The anonymous-ness is encoded in the names of the anonymous structs and the recommendation was to parse the names. See #99 (comment).

@mikebattista
Copy link
Contributor

Projections can parse the names to identify anonymous structs vs. named structs. See the native union notes at https://github.com/microsoft/win32metadata/blob/main/docs/projections.md for the predictable naming pattern.

@mikebattista mikebattista closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability Touch-up to improve the user experience for a language projection
Projects
None yet
Development

No branches or pull requests

6 participants