Move dunder new resolution to classes.rs

Summary: I'm looking at how to improve attribute get and set resolution, and for the most part general attribute reads flow through `attr.rs`, which makes for clear layering that makes refactors feasible. But there are a couple of spots where arbitrary parts of `AnswersSolver` are reaching directly into the guts of `classes.rs` - in particular the `Attribute` struct - which is harder to refactor. I think most of these cases are situations where we have a reasonable implementation, but the logic belongs directly in `classes.rs` with a tightly constrained interface, instead of relying on a low-level function that could potentially be used to fetch *any* attribute. This diff moves the `__new__` lookup logic, which is low level in that it wants to see the lineage of `__new__` and therefore cannot rely on the general lookup functions in `attr.rs`, to `classes.rs`. Since this was actually the *only* use of `get_class_attribute_with_targs` I just inlined the functionality, since at the moment I'm not aware of a single other use case where we actually need this kind of lineage to get desired behavior. Reviewed By: ndmitchell, rchen152 Differential Revision: D68599258 fbshipit-source-id: 437a3c01a64d0ee9f9acf754613aaf89e0625143
facebook · Jan 24, 2025 · 780b967 · 780b967
1 parent bec09d1
commit 780b967
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 20 deletions.
diff --git a/pyre2/pyre2/bin/alt/classes.rs b/pyre2/pyre2/bin/alt/classes.rs
@@ -32,6 +32,7 @@ use crate::binding::binding::Key;
 use crate::binding::binding::KeyClassField;
 use crate::binding::binding::KeyClassMetadata;
 use crate::binding::binding::KeyLegacyTypeParam;
+use crate::dunder;
 use crate::graph::index::Idx;
 use crate::module::short_identifier::ShortIdentifier;
 use crate::types::annotation::Annotation;
@@ -868,17 +869,20 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
         }
     }
 
-    /// Gets an attribute from a class with type arguments (i.e., a ClassType).
-    pub fn get_class_attribute_with_targs(
-        &self,
-        cls: &ClassType,
-        name: &Name,
-    ) -> Option<Attribute> {
-        self.get_class_member(cls.class_object(), name)
+    /// Get the class's `__new__` method.
+    pub fn get_dunder_new(&self, cls: &ClassType) -> Option<Type> {
+        let new_attr = self
+            .get_class_member(cls.class_object(), &dunder::NEW)
             .map(|(member, defining_class)| Attribute {
                 value: cls.instantiate_member(member.ty),
                 defining_class,
-            })
+            })?;
+        if new_attr.defined_on(self.stdlib.object_class_type().class_object()) {
+            // The default behavior of `object.__new__` is already baked into our implementation of
+            // class construction; we only care about `__new__` if it is overridden.
+            return None;
+        }
+        Some(new_attr.value)
     }
 
     /// Given an identifier, see whether it is bound to an enum class. If so,

diff --git a/pyre2/pyre2/bin/alt/expr.rs b/pyre2/pyre2/bin/alt/expr.rs
@@ -612,17 +612,6 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
         self.solver().expand(ret)
     }
 
-    /// Get the class's `__new__` method.
-    fn get_new(&self, cls: &ClassType) -> Option<Type> {
-        let new_attr = self.get_class_attribute_with_targs(cls, &dunder::NEW)?;
-        if new_attr.defined_on(self.stdlib.object_class_type().class_object()) {
-            // The default behavior of `object.__new__` is already baked into our implementation of
-            // class construction; we only care about `__new__` if it is overridden.
-            return None;
-        }
-        Some(new_attr.value)
-    }
-
     fn construct(
         &self,
         cls: ClassType,
@@ -640,7 +629,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
             // Got something other than an instance of the class under construction.
             return ret;
         }
-        let overrides_new = if let Some(new_method) = self.get_new(&cls) {
+        let overrides_new = if let Some(new_method) = self.get_dunder_new(&cls) {
             let cls_ty = Type::type_form(instance_ty.clone());
             let mut full_args = vec![CallArg::Type(&cls_ty, range)];
             full_args.extend_from_slice(args);