Skip to content

Commit

Permalink
Make ClassField internals private
Browse files Browse the repository at this point in the history
Summary:
I'm preparing for a major refactor where class attributes are
modeled in terms of get and set actions on instance vs class objects,
which is needed to handle
- static typing rules like `final` that allow get but nut set actions
- lots of built-in descriptors like `property` and `classmethod`
- eventually user-defined descriptors

The change is going to require rethinking `ClassField`, through which
all of the metadata needed for attribute actions must flow. One thing
that would easily get in the way is if Pyre2 as a whole is relying on
the internals of `ClassField`, which should not happen (that data
structure exists *only* for `classes.rs` to model how attribute access
actually works).

This diff enforces that by making all the internals of `ClassField`
private; the type itself is still public because `AnswersSolver` still
has to be aware of it (otherwise we couldn't write down the `Keyed`
instance for `BindingClassField`), but the bindings table doesn't need
to know anything about the internals.

Reviewed By: ndmitchell

Differential Revision: D68662438

fbshipit-source-id: f86f777bf67b89f64913fe897eb6bc4470496399
  • Loading branch information
stroxler authored and facebook-github-bot committed Jan 26, 2025
1 parent 9c03969 commit a351fd5
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 14 deletions.
18 changes: 7 additions & 11 deletions pyre2/pyre2/bin/alt/answers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,10 @@ impl SolveRecursive for KeyClassField {
fn promote_recursive(_: Self::Recursive) -> Self::Answer {
// TODO(stroxler) Revisit the recursive handling, which needs changes in the plumbing
// to work correctly; what we have here is a fallback to permissive gradual typing.
ClassField {
ty: Type::any_implicit(),
annotation: None,
initialization: ClassFieldInitialization::Class,
}
ClassField::recursive()
}
fn visit_type_mut(v: &mut ClassField, f: &mut dyn FnMut(&mut Type)) {
f(&mut v.ty);
v.visit_type_mut(f);
}
}
impl SolveRecursive for KeyAnnotation {
Expand Down Expand Up @@ -1041,11 +1037,11 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
} else {
(value_ty, None)
};
Arc::new(ClassField {
ty: ty.deref().clone(),
annotation: ann.map(|ann| ann.deref().clone()),
initialization: field.initialization,
})
Arc::new(ClassField::new(
ty.deref().clone(),
ann.map(|ann| ann.deref().clone()),
field.initialization,
))
}

fn solve_binding_inner(&self, binding: &Binding) -> Type {
Expand Down
32 changes: 29 additions & 3 deletions pyre2/pyre2/bin/alt/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,35 @@ use crate::util::prelude::SliceExt;
/// both visibility rules and whether method binding should be performed.
#[derive(Debug, Clone)]
pub struct ClassField {
pub ty: Type,
pub annotation: Option<Annotation>,
pub initialization: ClassFieldInitialization,
ty: Type,
annotation: Option<Annotation>,
initialization: ClassFieldInitialization,
}

impl ClassField {
pub fn new(
ty: Type,
annotation: Option<Annotation>,
initialization: ClassFieldInitialization,
) -> Self {
Self {
ty,
annotation,
initialization,
}
}

pub fn recursive() -> Self {
Self {
ty: Type::any_implicit(),
annotation: None,
initialization: ClassFieldInitialization::Class,
}
}

pub fn visit_type_mut(&mut self, f: &mut dyn FnMut(&mut Type)) {
f(&mut self.ty);
}
}

impl Display for ClassField {
Expand Down

0 comments on commit a351fd5

Please sign in to comment.