From fc3fa708b0d6ef22124be473c74b5424ce3ea7e8 Mon Sep 17 00:00:00 2001
From: kngwyu <yuji.kngw.80s.revive@gmail.com>
Date: Wed, 17 Jun 2020 12:43:12 +0900
Subject: [PATCH] Add more comments for #[pyproto] related parts

---
 pyo3-derive-backend/src/defs.rs    | 14 ++++++++++++++
 pyo3-derive-backend/src/pyclass.rs |  2 +-
 pyo3-derive-backend/src/pyproto.rs | 23 +++++++++++------------
 src/class/proto_methods.rs         |  4 ++--
 4 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/pyo3-derive-backend/src/defs.rs b/pyo3-derive-backend/src/defs.rs
index aa292edd..d7d73cfc 100644
--- a/pyo3-derive-backend/src/defs.rs
+++ b/pyo3-derive-backend/src/defs.rs
@@ -1,12 +1,19 @@
 // Copyright (c) 2017-present PyO3 Project and Contributors
 use crate::func::MethodProto;
 
+/// Predicates for `#[pyproto]`.
 pub struct Proto {
+    /// The name of this protocol. E.g., Iter.
     pub name: &'static str,
+    /// The name of slot table. E.g., PyIterMethods.
     pub slot_table: &'static str,
+    /// The name of the setter used to set the table to `PyProtoRegistry`.
     pub set_slot_table: &'static str,
+    /// All methods.
     pub methods: &'static [MethodProto],
+    /// All methods registered as normal methods like `#[pymethods]`.
     pub py_methods: &'static [PyMethod],
+    /// All methods registered to the slot table.
     pub slot_setters: &'static [SlotSetter],
 }
 
@@ -25,6 +32,7 @@ impl Proto {
     }
 }
 
+/// Represents a method registered as a normal method like `#[pymethods]`.
 // TODO(kngwyu): Currently only __radd__-like methods use METH_COEXIST to prevent
 // __add__-like methods from overriding them.
 pub struct PyMethod {
@@ -50,9 +58,15 @@ impl PyMethod {
     }
 }
 
+/// Represents a setter used to register a method to the method table.
 pub struct SlotSetter {
+    /// Protocols necessary for invoking this setter.
+    /// E.g., we need `__setattr__` and `__delattr__` for invoking `set_setdelitem`.
     pub proto_names: &'static [&'static str],
+    /// The name of the setter called to the method table.
     pub set_function: &'static str,
+    /// Represents a set of setters disabled by this setter.
+    /// E.g., `set_setdelitem` have to disable `set_setitem` and `set_delitem`.
     pub skipped_setters: &'static [&'static str],
 }
 
diff --git a/pyo3-derive-backend/src/pyclass.rs b/pyo3-derive-backend/src/pyclass.rs
index aeecf96a..251c9879 100644
--- a/pyo3-derive-backend/src/pyclass.rs
+++ b/pyo3-derive-backend/src/pyclass.rs
@@ -236,7 +236,7 @@ fn impl_methods_inventory(cls: &syn::Ident) -> TokenStream {
     }
 }
 
-/// TODO(kngwyu): doc
+/// Implement `HasProtoRegistry` for the class for lazy protocol initialization.
 fn impl_proto_registry(cls: &syn::Ident) -> TokenStream {
     quote! {
         impl pyo3::class::proto_methods::HasProtoRegistry for #cls {
diff --git a/pyo3-derive-backend/src/pyproto.rs b/pyo3-derive-backend/src/pyproto.rs
index 83ecf33e..1cded509 100644
--- a/pyo3-derive-backend/src/pyproto.rs
+++ b/pyo3-derive-backend/src/pyproto.rs
@@ -71,7 +71,7 @@ fn impl_proto_impl(
                 // Insert the method to the HashSet
                 method_names.insert(met.sig.ident.to_string());
             }
-            // Add non-slot methods to inventory slots
+            // Add non-slot methods to inventory like `#[pymethods]`
             if let Some(m) = proto.get_method(&met.sig.ident) {
                 let name = &met.sig.ident;
                 let fn_spec = FnSpec::parse(&met.sig, &mut met.attrs, false)?;
@@ -82,7 +82,7 @@ fn impl_proto_impl(
                 } else {
                     quote!(0)
                 };
-                // TODO(kngwyu): ml_doc
+                // TODO(kngwyu): Set ml_doc
                 py_methods.push(quote! {
                     pyo3::class::PyMethodDefType::Method({
                         #method
@@ -125,24 +125,23 @@ fn slot_initialization(
     ty: &syn::Type,
     proto: &defs::Proto,
 ) -> syn::Result<TokenStream> {
-    let mut initializers: Vec<TokenStream> = vec![];
     // Some setters cannot coexist.
     // E.g., if we have `__add__`, we need to skip `set_radd`.
-    let mut skipped_setters = HashSet::new();
-    'setter_loop: for m in proto.slot_setters {
-        if skipped_setters.contains(m.set_function) {
+    let mut skipped_setters = Vec::new();
+    // Collect initializers
+    let mut initializers: Vec<TokenStream> = vec![];
+    'outer_loop: for m in proto.slot_setters {
+        if skipped_setters.contains(&m.set_function) {
             continue;
         }
         for name in m.proto_names {
+            // If this `#[pyproto]` block doesn't provide all required methods,
+            // let's skip implementing this method.
             if !method_names.contains(*name) {
-                // If this `#[pyproto]` block doesn't provide all required methods,
-                // let's skip implementing this method.
-                continue 'setter_loop;
+                continue 'outer_loop;
             }
         }
-        for s in m.skipped_setters {
-            skipped_setters.insert(s.to_string());
-        }
+        skipped_setters.extend_from_slice(m.skipped_setters);
         // Add slot methods to PyProtoRegistry
         let set = syn::Ident::new(m.set_function, Span::call_site());
         initializers.push(quote! { table.#set::<#ty>(); });
diff --git a/src/class/proto_methods.rs b/src/class/proto_methods.rs
index 1266dcd0..76e75d9f 100644
--- a/src/class/proto_methods.rs
+++ b/src/class/proto_methods.rs
@@ -9,8 +9,8 @@ use std::{
     sync::atomic::{AtomicPtr, Ordering},
 };
 
-/// Defines all methods for object protocols.
-// Note: stub implementations are for rust-numpy. Please remove them.
+/// Defines all method tables we need for object protocols.
+// Note(kngwyu): default implementations are for rust-numpy. Please don't remove them.
 pub trait PyProtoMethods {
     fn async_methods() -> Option<NonNull<PyAsyncMethods>> {
         None