Skip to content

Commit c0c9b09

Browse files
Nero5023meta-codesync[bot]
authored andcommitted
Register TypeCompiledImplAsStarlarkValue with TypeMatcher
Summary: Add registration support for `TypeCompiledImplAsStarlarkValue<T: TypeMatcher>,` which is a special wrapper type that exposes `TypeMatcher` implementations as Starlark values. 1. **Marker Trait Introduction**: Introduce the `TypeMatcherRegistered` marker trait to indicate that a TypeMatcher type has been properly registered in the vtable registry. This ensures compile-time safety for serialization/deserialization. 2. All registration code is gated behind the `pagable` feature flag. support backwards compatibility 2. **New #[type_matcher] Derive Macro**: Create a new proc macro in starlark_derive that: * Implements the `TypeMatcherRegistered` marker trait for the annotated type * Automatically generates vtable registration code for `TypeCompiledImplAsStarlarkValue<Self>` * Works for `TypeMatcher` types without generic parameters. Will make it fail in next diff when it have generic parameters --- ## Some impl details 1. defined a `TypeMatcherBase` trait that `TypeMatcher` deps on. Base on the cfg pagable to add `TypeMatcherRegistered` or not. This make it easier to mantain. 2. add the `TypeMatcherRegistered` for `T` of `fn alloc` in `TypeMatcherAlloc` when cfg pagable. It is for better error message. - With this, error message: P2141029408 - Without this, error message: P2141030387 Reviewed By: cjhopman Differential Revision: D90875024 fbshipit-source-id: bbfe8b4c911a709eb2248f5fdcefb03bde8dbf58
1 parent c9880f0 commit c0c9b09

File tree

13 files changed

+294
-19
lines changed

13 files changed

+294
-19
lines changed

starlark/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,7 @@
439439
mod macros;
440440

441441
pub use starlark_derive::starlark_module;
442+
pub use starlark_derive::type_matcher;
442443
pub use starlark_syntax::Error;
443444
pub use starlark_syntax::ErrorKind;
444445
pub use starlark_syntax::Result;

starlark/src/pagable/vtable_registry.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,4 +242,36 @@ mod tests {
242242
let vt = vtable.unwrap();
243243
assert_eq!(vt.type_name, "list");
244244
}
245+
246+
#[test]
247+
fn test_type_compiled_non_generic_matcher_is_registered() {
248+
// IsAnyOf is a non-generic TypeMatcher, so TypeCompiledImplAsStarlarkValue<IsAnyOf>
249+
// should be registered by the #[type_matcher] macro.
250+
use crate::values::typing::type_compiled::compiled::TypeCompiledImplAsStarlarkValue;
251+
use crate::values::typing::type_compiled::matchers::IsAnyOf;
252+
let type_id = DeserTypeId::of::<TypeCompiledImplAsStarlarkValue<IsAnyOf>>();
253+
let vtable = lookup_vtable(type_id);
254+
assert!(
255+
vtable.is_ok(),
256+
"Expected TypeCompiledImplAsStarlarkValue<IsAnyOf> to be registered. Available types: {:?}",
257+
registered_type_ids()
258+
);
259+
}
260+
261+
#[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).
266+
use crate::values::typing::type_compiled::compiled::TypeCompiledImplAsStarlarkValue;
267+
use crate::values::typing::type_compiled::matcher::TypeMatcherBox;
268+
use crate::values::typing::type_compiled::matchers::IsListOf;
269+
let type_id =
270+
DeserTypeId::of::<TypeCompiledImplAsStarlarkValue<IsListOf<TypeMatcherBox>>>();
271+
let vtable = lookup_vtable(type_id);
272+
assert!(
273+
vtable.is_err(),
274+
"Expected TypeCompiledImplAsStarlarkValue<IsListOf<TypeMatcherBox>> to NOT be registered, but it was found"
275+
);
276+
}
245277
}

starlark/src/typing/structs.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ use std::sync::Arc;
2222

2323
use allocative::Allocative;
2424
use dupe::Dupe;
25+
use starlark_derive::type_matcher;
2526
use starlark_map::sorted_map::SortedMap;
2627

28+
use crate as starlark;
2729
use crate::typing::Ty;
2830
use crate::typing::TyBasic;
2931
use crate::typing::TypingBinOp;
@@ -37,6 +39,16 @@ use crate::values::structs::StructRef;
3739
use crate::values::typing::type_compiled::alloc::TypeMatcherAlloc;
3840
use crate::values::typing::type_compiled::matcher::TypeMatcher;
3941

42+
#[derive(Allocative, Eq, PartialEq, Hash, Debug, Clone, Copy, Dupe)]
43+
struct StructMatcher;
44+
45+
#[type_matcher]
46+
impl TypeMatcher for StructMatcher {
47+
fn matches(&self, value: Value) -> bool {
48+
StructRef::is_instance(value)
49+
}
50+
}
51+
4052
/// Struct type.
4153
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Allocative)]
4254
pub struct TyStruct {
@@ -109,15 +121,6 @@ impl TyCustomImpl for TyStruct {
109121
}
110122

111123
fn matcher<T: TypeMatcherAlloc>(&self, factory: T) -> T::Result {
112-
#[derive(Allocative, Eq, PartialEq, Hash, Debug, Clone, Copy, Dupe)]
113-
struct StructMatcher;
114-
115-
impl TypeMatcher for StructMatcher {
116-
fn matches(&self, value: Value) -> bool {
117-
StructRef::is_instance(value)
118-
}
119-
}
120-
121124
factory.alloc(StructMatcher)
122125
}
123126
}

starlark/src/values/types/enumeration/matcher.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717

1818
use allocative::Allocative;
1919
use dupe::Dupe;
20+
use starlark_derive::type_matcher;
2021

22+
use crate as starlark;
2123
use crate::values::Value;
2224
use crate::values::enumeration::EnumValue;
2325
use crate::values::types::type_instance_id::TypeInstanceId;
@@ -28,6 +30,7 @@ pub(crate) struct EnumTypeMatcher {
2830
pub(crate) id: TypeInstanceId,
2931
}
3032

33+
#[type_matcher]
3134
impl TypeMatcher for EnumTypeMatcher {
3235
fn matches(&self, value: Value) -> bool {
3336
match EnumValue::from_value(value) {

starlark/src/values/types/namespace/typing.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ use std::fmt::Formatter;
2121

2222
use allocative::Allocative;
2323
use dupe::Dupe;
24+
use starlark_derive::type_matcher;
2425
use starlark_map::sorted_map::SortedMap;
2526

27+
use crate as starlark;
2628
use crate::codemap::Span;
2729
use crate::typing::ParamSpec;
2830
use crate::typing::Ty;
@@ -40,6 +42,16 @@ use crate::values::types::namespace::value::Namespace;
4042
use crate::values::typing::type_compiled::alloc::TypeMatcherAlloc;
4143
use crate::values::typing::type_compiled::matcher::TypeMatcher;
4244

45+
#[derive(Allocative, Eq, PartialEq, Hash, Debug, Clone, Copy, Dupe)]
46+
struct NamespaceMatcher;
47+
48+
#[type_matcher]
49+
impl TypeMatcher for NamespaceMatcher {
50+
fn matches(&self, value: Value) -> bool {
51+
value.starlark_type_id() == StarlarkTypeId::of::<Namespace<'static>>()
52+
}
53+
}
54+
4355
#[derive(
4456
Allocative, Clone, Copy, Dupe, Debug, Eq, PartialEq, Hash, Ord, PartialOrd
4557
)]
@@ -106,15 +118,6 @@ impl TyCustomImpl for TyNamespace {
106118
}
107119

108120
fn matcher<T: TypeMatcherAlloc>(&self, factory: T) -> T::Result {
109-
#[derive(Allocative, Eq, PartialEq, Hash, Debug, Clone, Copy, Dupe)]
110-
struct NamespaceMatcher;
111-
112-
impl TypeMatcher for NamespaceMatcher {
113-
fn matches(&self, value: Value) -> bool {
114-
value.starlark_type_id() == StarlarkTypeId::of::<Namespace<'static>>()
115-
}
116-
}
117-
118121
factory.alloc(NamespaceMatcher)
119122
}
120123
}

starlark/src/values/types/record/matcher.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717

1818
use allocative::Allocative;
1919
use dupe::Dupe;
20+
use starlark_derive::type_matcher;
2021

22+
use crate as starlark;
2123
use crate::values::Value;
2224
use crate::values::record::Record;
2325
use crate::values::types::type_instance_id::TypeInstanceId;
@@ -28,6 +30,7 @@ pub(crate) struct RecordTypeMatcher {
2830
pub(crate) id: TypeInstanceId,
2931
}
3032

33+
#[type_matcher]
3134
impl TypeMatcher for RecordTypeMatcher {
3235
fn matches(&self, value: Value) -> bool {
3336
match Record::from_value(value) {

starlark/src/values/typing.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ pub use crate::values::typing::callable::param::StarlarkCallableParamSpec;
3636
pub use crate::values::typing::iter::StarlarkIter;
3737
pub use crate::values::typing::never::StarlarkNever;
3838
pub use crate::values::typing::type_compiled::compiled::TypeCompiled;
39+
pub use crate::values::typing::type_compiled::compiled::TypeCompiledImplAsStarlarkValue;
3940
pub use crate::values::typing::type_compiled::matcher::TypeMatcher;
41+
pub use crate::values::typing::type_compiled::matcher::TypeMatcherRegistered;
4042
pub use crate::values::typing::type_compiled::type_matcher_factory::TypeMatcherFactory;
4143
pub use crate::values::typing::type_type::TypeType;

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,19 @@ use crate::values::typing::type_compiled::type_matcher_factory::TypeMatcherFacto
4747
pub trait TypeMatcherAlloc: Sized {
4848
type Result;
4949

50+
// When the `pagable` feature is enabled, we explicitly require `TypeMatcherRegistered`,
51+
// although it is reuqired by TypeMatcher.
52+
// It can produce better error messages. Generic matchers that not impls TypeMatcherRegistered
53+
// will have concrete type in the error messages.
54+
#[cfg(feature = "pagable")]
55+
fn alloc<
56+
T: TypeMatcher + crate::values::typing::type_compiled::matcher::TypeMatcherRegistered,
57+
>(
58+
self,
59+
matcher: T,
60+
) -> Self::Result;
61+
62+
#[cfg(not(feature = "pagable"))]
5063
fn alloc<T: TypeMatcher>(self, matcher: T) -> Self::Result;
5164

5265
fn custom(self, custom: &TyCustom) -> Self::Result;

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use allocative::Allocative;
2626
use dupe::Dupe;
2727
use starlark_derive::starlark_module;
2828
use starlark_derive::starlark_value;
29+
use starlark_derive::type_matcher;
2930
use starlark_map::StarlarkHasher;
3031
use starlark_syntax::slice_vec_ext::SliceExt;
3132
use starlark_syntax::slice_vec_ext::VecExt;
@@ -119,6 +120,7 @@ where
119120
ProvidesStaticType,
120121
NoSerialize
121122
)]
123+
/// A compiled type expression wrapped as a Starlark value with a type matcher.
122124
pub struct TypeCompiledImplAsStarlarkValue<T: 'static> {
123125
type_compiled_impl: T,
124126
ty: Ty,
@@ -143,6 +145,7 @@ where
143145
#[derive(Hash, Eq, PartialEq, Debug, Clone, Allocative)]
144146
pub struct DummyTypeMatcher;
145147

148+
#[type_matcher]
146149
impl TypeMatcher for DummyTypeMatcher {
147150
fn matches(&self, _value: Value) -> bool {
148151
unreachable!()

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

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,52 @@
1818
use std::fmt::Debug;
1919

2020
use allocative::Allocative;
21+
use starlark_derive::type_matcher;
2122

23+
use crate as starlark;
2224
use crate::typing::custom::TyCustom;
2325
use crate::values::Value;
2426
use crate::values::typing::type_compiled::alloc::TypeMatcherAlloc;
2527
use crate::values::typing::type_compiled::type_matcher_factory::TypeMatcherFactory;
2628

29+
/// Marker trait for type matchers which are registered.
30+
///
31+
/// This trait is automatically implemented by the `#[type_matcher]` proc macro.
32+
///
33+
/// # Safety
34+
///
35+
/// This trait must only be implemented by the `#[type_matcher]` proc macro,
36+
/// which ensures the type is properly registered in the vtable registry.
37+
/// Manual implementations may break deserialization.
38+
#[cfg_attr(not(feature = "pagable"), allow(dead_code))]
39+
pub unsafe trait TypeMatcherRegistered {}
40+
41+
/// Base trait for type matchers
42+
///
43+
/// When `pagable` is enabled, matchers must also implement `TypeMatcherRegistered`
44+
/// to ensure they are registered.
45+
#[cfg(feature = "pagable")]
46+
pub trait TypeMatcherBase:
47+
TypeMatcherRegistered + Allocative + Debug + Clone + Sized + Send + Sync + 'static
48+
{
49+
}
50+
51+
#[cfg(feature = "pagable")]
52+
impl<T> TypeMatcherBase for T where
53+
T: TypeMatcherRegistered + Allocative + Debug + Clone + Sized + Send + Sync + 'static
54+
{
55+
}
56+
57+
/// Base trait for type matchers
58+
#[cfg(not(feature = "pagable"))]
59+
pub trait TypeMatcherBase: Allocative + Debug + Clone + Sized + Send + Sync + 'static {}
60+
61+
#[cfg(not(feature = "pagable"))]
62+
impl<T> TypeMatcherBase for T where T: Allocative + Debug + Clone + Sized + Send + Sync + 'static {}
63+
2764
/// Runtime type matcher. E.g. when `isinstance(1, int)` is called,
2865
/// implementation of `TypeMatcher` for `int` is used.
29-
pub trait TypeMatcher: Allocative + Debug + Clone + Sized + Send + Sync + 'static {
66+
pub trait TypeMatcher: TypeMatcherBase {
3067
/// Check if the value matches the type.
3168
fn matches(&self, value: Value) -> bool;
3269
/// True if this matcher matches any value.
@@ -71,6 +108,7 @@ impl Clone for TypeMatcherBox {
71108
}
72109
}
73110

111+
#[type_matcher]
74112
impl TypeMatcher for TypeMatcherBox {
75113
fn matches(&self, value: Value) -> bool {
76114
self.0.matches_dyn(value)

0 commit comments

Comments
 (0)