Skip to content

Fix windows-rdl complex attribute refs - #3942

Merged
Kenny Kerr (kennykerr) merged 7 commits into
masterfrom
copilot/fix-windows-rdl-complex-attributes
Mar 6, 2026
Merged

Fix windows-rdl complex attribute refs#3942
Kenny Kerr (kennykerr) merged 7 commits into
masterfrom
copilot/fix-windows-rdl-complex-attributes

Conversation

Copilot AI commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Fixes several bugs that prevented Windows.Foundation.Metadata attribute refs from working correctly on non-class types and with path-typed constructor arguments.

  • Reader (attribute_ref.rs): encode_attr_value now only accepts Rust path expressions (IStatics, Test::IStatics, etc.) for System.Type constructor params — string literals are rejected. Paths are resolved via encode_path and stored as the new Value::TypeName variant. The AttributeRef.types workaround field has been removed; the MemberRef signature is now derived directly from Value::ty(), which returns Type::Name(TypeName::named("System", "Type")) for Value::TypeName.

  • windows-metadata Value enum: Added a TypeName(TypeName) variant to explicitly represent type-name values rather than encoding them as ambiguous UTF-8 strings. read_value now produces Value::TypeName when reading System.Type blobs (splitting the stored dotted string into namespace + name). The binary writer serialises Value::TypeName back to the same dotted UTF-8 wire format, so existing .winmd files are unaffected.

  • windows-bindgen (class.rs): StaticAttribute, ActivatableAttribute, and ComposableAttribute handling now matches Value::TypeName exclusively when looking up interface type names — the old Value::Utf8 dotted-string path is no longer needed since System.Type attribute params are always represented as Value::TypeName.

  • Writer (writer/mod.rs): write_custom_attributes blanket-skipped all Windows.Foundation.Metadata attributes, silently dropping them on roundtrip for structs and other non-class types. Now only System.* (CLR-internal) attributes are unconditionally skipped; Windows.Foundation.Metadata attributes respect the caller's explicit skip list (class behaviour unchanged). External-namespace attributes are emitted with a fully-qualified path (e.g. #[Windows::Foundation::Metadata::Activatable(3)]). Value::TypeName args are emitted as type paths via write_type, making roundtrips lossless. write_value now takes a namespace parameter used to produce relative type paths.

  • Formatter (formatter/mod.rs): push_attribute now uses a peekable iterator to detect ::, trims the space before it, and suppresses the space after via an after_colon_colon flag, so qualified attribute paths are formatted correctly (e.g. Windows::Foundation::Metadata::Activatable(3) with no extra spaces).

  • Path casing (attribute-from-reference.rs, nested.rs, path.rs): Tests referenced windows.winmd (lowercase) while the actual file is Windows.winmd, silently failing on case-sensitive Linux filesystems.

Example that now works end-to-end:

#[Windows::Foundation::Metadata::Activatable(3)]
#[Windows::Foundation::Metadata::Static(IStatics, 4)]
struct MyStruct {
    x: i32,
}
Original prompt

This section details on the original issue you should resolve

<issue_title>windows-rdl complex attributes refs</issue_title>
<issue_description>### Suggestion

#3940 added support for attribute refs, but the following complex test fails.

  1. The Activatable attribute lands up properly encoded in the .winmd but the Writer fails to emit it, thus failing to roundtrip.

  2. The Static attribute claims "no matching attribute constructor found" but StaticAttribute has a ctor that accepts Type, u32 arguments.

#[winrt]
mod Test {
    attribute FirstAttribute {
        fn(value: u32);
    }

    attribute SecondAttribute {
        fn(value: u32);
    }

    interface IStatics {}

    #[First(1)]
    #[Second(2)]
    #[Windows::Foundation::Metadata::Activatable(3)]
    #[Windows::Foundation::Metadata::Static(IStatics, 4)]
    struct MyStruct {
        x: i32,
    }
}

</issue_description>

Comments on the Issue (you are Copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…ter WFM emit)

Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix encoding issues with complex attributes in windows-rdl Fix windows-rdl complex attribute refs Mar 6, 2026
interface IStatics {}
#[First(1)]
#[Second(2)]
#[Windows :: Foundation :: Metadata :: Activatable(3)]

@kennykerr Kenny Kerr (kennykerr) Mar 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot The extra spaces around :: should be removed during formatting.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot The extra spaces around :: should be removed during formatting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8e4cfa8. push_attribute in the formatter now uses a peekable iterator to detect ::, trims the space before it, emits :: directly, and suppresses the space after it via an after_colon_colon flag — so Windows::Foundation::Metadata::Activatable(3) is now emitted without extra spaces.

#[First(1)]
#[Second(2)]
#[Windows :: Foundation :: Metadata :: Activatable(3)]
#[Windows :: Foundation :: Metadata :: Static("Test.IStatics", 4)]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot The Static("Test.IStatics", 4) should be written as Static(IStatics, 4) or Static(Test::IStatics, 4)` since it is a type rather than a string.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8e4cfa8. The root cause was that Value::Utf8.ty() returns Type::String, so the MemberRef was written with a String signature parameter instead of System.Type. When reading back, the writer's type-path check never matched. The fix adds a types field to AttributeRef that captures the matched constructor's parameter types, which are then used directly in encode_named_attribute to build the MemberRef signature. Now Static(IStatics, 4) roundtrips cleanly.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot this seems a bit contrived - would it not be simpler to add a Name(TypeName) variant to the windows-metadata crate's Value enum so that we can be explicit about the fact that the "value" represents a type name rather than just a string?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 75dc740. Added Value::TypeName(TypeName) to the windows-metadata Value enum. ty() returns Type::Name(TypeName::named("System", "Type")) so the MemberRef signature is derived automatically. read_value now produces Value::TypeName for System.Type blobs, write_value (helpers.rs) serializes it back as a dotted UTF-8 string, and the RDL writer emits it as a type path via write_type. The AttributeRef.types workaround field is gone — everything flows through the type system cleanly.

Comment on lines 185 to 188
@@ -186,7 +186,15 @@ fn encode_attr_value(
lit: syn::Lit::Str(s),
..
}) => Ok(metadata::Value::Utf8(s.value())),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot Type parameters should not accept strings, just paths.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2dac884. The string literal arm for System.Type has been removed — only path expressions are accepted now, with the error message updated to "expected type path".

…ture

Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
… only

Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
…e attribute values

Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
@kennykerr

Copy link
Copy Markdown
Collaborator

Copilot fix multiple build breaks pls

Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>

Copilot AI commented Mar 6, 2026

Copy link
Copy Markdown
Contributor Author

Copilot fix multiple build breaks pls

Fixed in 259ce16. Two failures:

  • fmt: Reformatted the nested match in attribute_ref.rs to rustfmt style (flat arm instead of braced block).
  • gen: class.rs in windows-bindgen only matched Value::Utf8 when looking up interface types from StaticAttribute/ActivatableAttribute/ComposableAttribute. Since Value::TypeName is now returned for System.Type params, those lookups silently failed — no factory/statics interfaces were found, so factory methods like CreateUri were dropped. Both arms now match: Value::TypeName(tn) uses the namespace/name directly, Value::Utf8(s) falls back to the dotted-string split.

Comment thread crates/libs/bindgen/src/types/class.rs Outdated
Comment on lines 331 to 336
Value::Utf8(s) => {
if let Some(dot) = s.rfind('.') {
(&s[..dot], &s[dot + 1..])
} else {
continue;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot this should always be a TypeName so perhaps don't bother with checking for a Utf8

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 6643f3f. Both loops now only match Value::TypeName, dropping the Value::Utf8 fallback entirely.

…Composable type params

Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
@kennykerr
Kenny Kerr (kennykerr) merged commit 7de2436 into master Mar 6, 2026
38 checks passed
@kennykerr
Kenny Kerr (kennykerr) deleted the copilot/fix-windows-rdl-complex-attributes branch March 6, 2026 03:33
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

Successfully merging this pull request may close these issues.

windows-rdl complex attributes refs

2 participants