Skip to content

Commit f105e1d

Browse files
Nero5023meta-codesync[bot]
authored andcommitted
Handle generic type parameters in TypeMatcher registration
Summary: Handle generic type parameters in TypeMatcher registration 1. The `#[type_matcher]` macro now raises a compile-time error when applied to a type with generic parameters. The error has the instruction to how to handle the issue. So ai agent can follow it to resolve it. 2. For generic types, developers must manually register concrete instantiations by: 1. From the error message of rust check to discover needed registrations 2. Adding a blanket `unsafe impl TypeMatcherRegistered` 3. Adding `register_type_matcher!` for each concrete instantiation Reviewed By: cjhopman Differential Revision: D90875026 fbshipit-source-id: 384d06a7f82ec6d12e6a5ed80ce0c18150dc3f8b
1 parent c0c9b09 commit f105e1d

File tree

4 files changed

+95
-27
lines changed

4 files changed

+95
-27
lines changed

starlark/src/pagable/vtable_register.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,3 +62,26 @@ macro_rules! register_special_avalue_frozen {
6262
}
6363

6464
pub(crate) use register_special_avalue_frozen;
65+
66+
/// Macro to register a vtable for a generic `TypeMatcher` implementation.
67+
///
68+
/// This macro is for generic `TypeMatcher` types (like `IsListOf<TypeMatcherBox>`,
69+
/// `IsDictOf<TypeMatcherBox, TypeMatcherBox>`) that cannot use the `#[type_matcher]`
70+
/// attribute macro (which only supports non-generic types).
71+
///
72+
/// # Example
73+
///
74+
/// ```ignore
75+
/// register_type_matcher!(IsTupleOf<TypeMatcherBox>);
76+
/// ```
77+
#[macro_export]
78+
macro_rules! register_type_matcher {
79+
($matcher:ty) => {
80+
#[cfg(feature = "pagable")]
81+
$crate::register_avalue_simple_frozen!(
82+
$crate::values::typing::TypeCompiledImplAsStarlarkValue<
83+
$matcher,
84+
>
85+
);
86+
};
87+
}

starlark/src/pagable/vtable_registry.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -259,19 +259,19 @@ mod tests {
259259
}
260260

261261
#[test]
262-
fn test_type_compiled_generic_matcher_is_not_registered() {
263-
// IsListOf is a generic TypeMatcher (IsListOf<I>), so TypeCompiledImplAsStarlarkValue
264-
// for specific instantiations like IsListOf<TypeMatcherBox> should NOT be registered
265-
// (the macro skips vtable registration for generic types).
262+
fn test_type_compiled_generic_matcher_is_registered() {
263+
// IsListOf is a generic TypeMatcher (IsListOf<I>). Specific instantiations
264+
// are registered via register_type_matcher! (e.g., IsListOf<TypeMatcherBox>).
266265
use crate::values::typing::type_compiled::compiled::TypeCompiledImplAsStarlarkValue;
267266
use crate::values::typing::type_compiled::matcher::TypeMatcherBox;
268267
use crate::values::typing::type_compiled::matchers::IsListOf;
269268
let type_id =
270269
DeserTypeId::of::<TypeCompiledImplAsStarlarkValue<IsListOf<TypeMatcherBox>>>();
271270
let vtable = lookup_vtable(type_id);
272271
assert!(
273-
vtable.is_err(),
274-
"Expected TypeCompiledImplAsStarlarkValue<IsListOf<TypeMatcherBox>> to NOT be registered, but it was found"
272+
vtable.is_ok(),
273+
"Expected TypeCompiledImplAsStarlarkValue<IsListOf<TypeMatcherBox>> to be registered. Available types: {:?}",
274+
registered_type_ids()
275275
);
276276
}
277277
}

starlark/src/values/typing/type_compiled/matchers.rs

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use dupe::Dupe;
2020
use starlark_derive::type_matcher;
2121

2222
use crate as starlark;
23+
use crate::register_type_matcher;
2324
use crate::typing::starlark_value::TyStarlarkValue;
2425
use crate::values::UnpackValue;
2526
use crate::values::Value;
@@ -35,6 +36,7 @@ use crate::values::tuple::value::Tuple;
3536
use crate::values::types::int::int_or_big::StarlarkIntRef;
3637
use crate::values::typing::type_compiled::matcher::TypeMatcher;
3738
use crate::values::typing::type_compiled::matcher::TypeMatcherBox;
39+
use crate::values::typing::type_compiled::matcher::TypeMatcherRegistered;
3840

3941
#[derive(Clone, Copy, Dupe, Allocative, Debug)]
4042
pub(crate) struct IsAny;
@@ -83,7 +85,11 @@ impl TypeMatcher for IsList {
8385
#[derive(Clone, Allocative, Debug)]
8486
pub(crate) struct IsListOf<I: TypeMatcher>(pub(crate) I);
8587

86-
#[type_matcher]
88+
unsafe impl<I: TypeMatcher> TypeMatcherRegistered for IsListOf<I> {}
89+
register_type_matcher!(IsListOf<IsStr>);
90+
register_type_matcher!(IsListOf<StarlarkTypeIdMatcher>);
91+
register_type_matcher!(IsListOf<TypeMatcherBox>);
92+
8793
impl<I: TypeMatcher> TypeMatcher for IsListOf<I> {
8894
fn matches(&self, value: Value) -> bool {
8995
match ListRef::from_value(value) {
@@ -96,7 +102,10 @@ impl<I: TypeMatcher> TypeMatcher for IsListOf<I> {
96102
#[derive(Clone, Allocative, Debug)]
97103
pub(crate) struct IsTupleOf<A: TypeMatcher>(pub(crate) A);
98104

99-
#[type_matcher]
105+
unsafe impl<A: TypeMatcher> TypeMatcherRegistered for IsTupleOf<A> {}
106+
register_type_matcher!(IsTupleOf<StarlarkTypeIdMatcher>);
107+
register_type_matcher!(IsTupleOf<TypeMatcherBox>);
108+
100109
impl<A: TypeMatcher> TypeMatcher for IsTupleOf<A> {
101110
fn matches(&self, value: Value) -> bool {
102111
match Tuple::from_value(value) {
@@ -137,7 +146,9 @@ impl TypeMatcher for IsTupleElems0 {
137146
#[derive(Clone, Allocative, Debug)]
138147
pub(crate) struct IsTupleElems1<A: TypeMatcher>(pub(crate) A);
139148

140-
#[type_matcher]
149+
unsafe impl<A: TypeMatcher> TypeMatcherRegistered for IsTupleElems1<A> {}
150+
register_type_matcher!(IsTupleElems1<TypeMatcherBox>);
151+
141152
impl<A: TypeMatcher> TypeMatcher for IsTupleElems1<A> {
142153
fn matches(&self, value: Value) -> bool {
143154
match Tuple::from_value(value).map(|t| t.content()) {
@@ -150,7 +161,10 @@ impl<A: TypeMatcher> TypeMatcher for IsTupleElems1<A> {
150161
#[derive(Clone, Allocative, Debug)]
151162
pub(crate) struct IsTupleElems2<A: TypeMatcher, B: TypeMatcher>(pub(crate) A, pub(crate) B);
152163

153-
#[type_matcher]
164+
unsafe impl<A: TypeMatcher, B: TypeMatcher> TypeMatcherRegistered for IsTupleElems2<A, B> {}
165+
register_type_matcher!(IsTupleElems2<StarlarkTypeIdMatcher, StarlarkTypeIdMatcher>);
166+
register_type_matcher!(IsTupleElems2<TypeMatcherBox, TypeMatcherBox>);
167+
154168
impl<A: TypeMatcher, B: TypeMatcher> TypeMatcher for IsTupleElems2<A, B> {
155169
fn matches(&self, value: Value) -> bool {
156170
match Tuple::from_value(value).map(|t| t.content()) {
@@ -173,7 +187,15 @@ impl TypeMatcher for IsDict {
173187
#[derive(Clone, Allocative, Debug)]
174188
pub(crate) struct IsDictOf<K: TypeMatcher, V: TypeMatcher>(pub(crate) K, pub(crate) V);
175189

176-
#[type_matcher]
190+
unsafe impl<K: TypeMatcher, V: TypeMatcher> TypeMatcherRegistered for IsDictOf<K, V> {}
191+
register_type_matcher!(IsDictOf<IsStr, TypeMatcherBox>);
192+
register_type_matcher!(IsDictOf<StarlarkTypeIdMatcher, TypeMatcherBox>);
193+
register_type_matcher!(IsDictOf<TypeMatcherBox, TypeMatcherBox>);
194+
register_type_matcher!(IsDictOf<IsAny, TypeMatcherBox>);
195+
register_type_matcher!(IsDictOf<TypeMatcherBox, IsAny>);
196+
register_type_matcher!(IsDictOf<IsStr, IsAny>);
197+
register_type_matcher!(IsDictOf<StarlarkTypeIdMatcher, IsAny>);
198+
177199
impl<K: TypeMatcher, V: TypeMatcher> TypeMatcher for IsDictOf<K, V> {
178200
fn matches(&self, value: Value) -> bool {
179201
match DictRef::from_value(value) {
@@ -198,7 +220,11 @@ impl TypeMatcher for IsSet {
198220
#[derive(Clone, Allocative, Debug)]
199221
pub(crate) struct IsSetOf<I: TypeMatcher>(pub(crate) I);
200222

201-
#[type_matcher]
223+
unsafe impl<I: TypeMatcher> TypeMatcherRegistered for IsSetOf<I> {}
224+
register_type_matcher!(IsSetOf<IsStr>);
225+
register_type_matcher!(IsSetOf<StarlarkTypeIdMatcher>);
226+
register_type_matcher!(IsSetOf<TypeMatcherBox>);
227+
202228
impl<I: TypeMatcher> TypeMatcher for IsSetOf<I> {
203229
fn matches(&self, value: Value) -> bool {
204230
match SetRef::unpack_value_opt(value) {
@@ -211,7 +237,14 @@ impl<I: TypeMatcher> TypeMatcher for IsSetOf<I> {
211237
#[derive(Clone, Allocative, Debug)]
212238
pub(crate) struct IsAnyOfTwo<A: TypeMatcher, B: TypeMatcher>(pub(crate) A, pub(crate) B);
213239

214-
#[type_matcher]
240+
unsafe impl<A: TypeMatcher, B: TypeMatcher> TypeMatcherRegistered for IsAnyOfTwo<A, B> {}
241+
register_type_matcher!(IsAnyOfTwo<IsNone, IsStr>);
242+
register_type_matcher!(IsAnyOfTwo<IsNone, IsInt>);
243+
register_type_matcher!(IsAnyOfTwo<IsNone, StarlarkTypeIdMatcher>);
244+
register_type_matcher!(IsAnyOfTwo<IsNone, IsList>);
245+
register_type_matcher!(IsAnyOfTwo<IsNone, TypeMatcherBox>);
246+
register_type_matcher!(IsAnyOfTwo<TypeMatcherBox, TypeMatcherBox>);
247+
215248
impl<A: TypeMatcher, B: TypeMatcher> TypeMatcher for IsAnyOfTwo<A, B> {
216249
fn matches(&self, value: Value) -> bool {
217250
self.0.matches(value) || self.1.matches(value)

starlark_derive/src/type_matcher.rs

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -80,32 +80,44 @@ fn derive_type_matcher_impl(_attr: TokenStream, input: TokenStream) -> syn::Resu
8080

8181
// Get the generics from the impl block
8282
let generics = &item_impl.generics;
83-
let (impl_generics, _, where_clause) = generics.split_for_impl();
8483

8584
// Check if the impl block has generic type parameters (not just lifetimes)
8685
let has_type_params = generics.type_params().next().is_some();
8786

87+
// Generic types are not supported - they require special handling for vtable registration
88+
if has_type_params {
89+
return Err(syn::Error::new(
90+
generics.span(),
91+
"#[type_matcher] does not support generic type parameters. \
92+
For generic TypeMatcher implementations: \
93+
1. Remove #[type_matcher] attribute from the impl block, \
94+
2. Run build or `arc rust-check` with `pagable` feature to find call sites, \
95+
3. Errors show types like `YourType<X>: TypeMatcherRegistered is not satisfied`. \
96+
Two cases for X: \
97+
a) Concrete type (e.g., `IsStr`, `IsNone`, `StarlarkTypeIdMatcher`) - use directly \
98+
b) `impl TypeMatcher` - trace CALLERS to find concrete type, usually `TypeMatcherBox` \
99+
from `TypeMatcherBoxAlloc.ty(...)` \
100+
4. Add blanket unsafe impl: `unsafe impl<...> TypeMatcherRegistered for YourType<...> {}`, \
101+
5. Add `register_type_matcher!(YourType<ConcreteArg>);` for each concrete instantiation. \
102+
Example: IsListOf errors show IsListOf<IsStr> (case a) and IsListOf<impl TypeMatcher> (case b). \
103+
Register both: `register_type_matcher!(IsListOf<IsStr>);` \
104+
`register_type_matcher!(IsListOf<TypeMatcherBox>);`",
105+
));
106+
}
107+
88108
// Generate TypeMatcherRegistered impl for both generic and non-generic types.
89-
// TODO(nero): Only generate this impl for non-generic types.
90109
let registered_impl = quote! {
91110
// SAFETY: This impl is generated by the #[type_matcher] macro.
92111
// For non-generic types, the vtable is also registered below.
93-
unsafe impl #impl_generics starlark::values::typing::TypeMatcherRegistered
112+
unsafe impl starlark::values::typing::TypeMatcherRegistered
94113
for #self_ty
95-
#where_clause
96114
{}
97115
};
98116

99-
// Generate vtable registration only for non-generic types.
100-
let vtable_registration = if has_type_params {
101-
// Generic type - skip vtable registration
102-
TokenStream::new()
103-
} else {
104-
quote! {
105-
starlark::register_avalue_simple_frozen!(
106-
starlark::values::typing::TypeCompiledImplAsStarlarkValue<#self_ty>
107-
);
108-
}
117+
// Generate vtable registration
118+
let vtable_registration = quote! {
119+
120+
starlark::register_type_matcher!(#self_ty);
109121
};
110122

111123
// Return the original impl block plus the generated code

0 commit comments

Comments
 (0)