Carry Atom in PhpType::Named

Author: AJenbo

docs/todo/eager-resolution.md

@@ -349,28 +349,44 @@ The problem is needing them for every runtime analysis thread.
 - No stack overflows on the full test suite or the largest available
   project.
 
-#### Phase 4g: Carry `Atom` in `PhpType::Named`
-
-**Impact: Low-Medium. Effort: Medium.**
-
-Deferred from Phase 4d. With the FQN now cached, the residual
-per-substitution string cost is `PhpType::Named(cls.fqn().to_string())`
-(and the equivalent patterns in `template_subs.rs`, `return_types.rs`,
-`inheritance/mod.rs`, `forward_walk/scope_state.rs`,
-`rhs_resolution/mod.rs`, and the Laravel builder providers). Each of
-these turns a cached `Atom` back into an owned `String` because
-`PhpType::Named` holds a `String`.
-
-Changing `PhpType::Named` (and the analogous per-member cache keys such
-as `format!("{}::{}", class_fqn, method.name)` in `target_cache.rs` /
-`property_access.rs`) to carry `Atom` would let the resolution pipeline
-thread interned names end-to-end without re-allocating. This touches a
-core enum used throughout the type engine, so it is a standalone
-refactor rather than part of the 4d fqn-cache change.
+#### Phase 4g: Carry `Atom` in `PhpType::Named` ✓
+
+Changed `PhpType::Named` to hold an `Atom` (interned string) instead of
+a `String`. `Named` values are drawn from a bounded set (class names,
+keywords, template parameters, `$this`) and are compared and cloned
+throughout the substitution hot path, so pointer-sized equality and
+free (`Copy`) cloning pay off; literal scalar values live in
+`PhpType::Literal`, so the interner is never fed unbounded free text.
+The substitution sites the FQN cache (Phase 4d) fed back into a fresh
+`String` — `PhpType::Named(cls.fqn().to_string())` in `template_subs.rs`,
+`return_types.rs`, `inheritance/mod.rs`, `rhs_resolution/mod.rs`, and the
+Laravel providers — now thread the cached `Atom` end-to-end
+(`PhpType::Named(cls.fqn())`) with no reallocation. `Atom`'s
+`Deref<Target = str>` means the ~160 read sites (comparisons, keyword
+checks, `base_name` extraction) were unaffected; only genuine
+`String`-typed sinks (`Vec<String>` keys, `Generic`'s `String` field)
+needed an explicit `.to_string()`.
+
+#### Phase 4h: Intern composite per-member cache keys
+
+**Impact: Low. Effort: Low.**
+
+`target_cache.rs` (`BODY_INFER_MEMO`, the re-entry set) and
+`property_access.rs` key thread-local caches on
+`format!("{}::{}", class_fqn, method)`. These allocate a `String` per
+lookup. Interning them into a single `Atom` key would remove the
+allocation, but the key space is the `FQN × member` cartesian product,
+which is effectively unbounded — feeding it to the global `ustr`
+interner would leak one entry per distinct pair for the process
+lifetime (the exact "constructed once, never compared" case
+`atom.rs` warns against). Any fix here should therefore use a
+composite `(Atom, Atom)` tuple key (both halves already interned,
+no new interner entries) rather than interning the joined string.
 
 **Success criteria:**
-- `PhpType::Named` carries an interned name (no `String`
-  reallocation on the substitution hot path).
+- The `"FQN::member"` cache keys no longer allocate a `String` per
+  lookup.
+- No net growth in the global interner.
 - No test regressions.
 
 ---

src/class_lookup.rs

@@ -7,6 +7,7 @@
 //! in [`crate::resolution`]; this module is the shared kernel that
 //! resolution and the completion/diagnostics pipelines both call into.
 
+use crate::atom::atom;
 use std::sync::Arc;
 
 use crate::types::ClassInfo;
@@ -263,7 +264,7 @@ pub(crate) fn is_subtype_of(
 /// class names instead of pre-constructed [`crate::php_type::PhpType`] values.
 ///
 /// This avoids the boilerplate of wrapping each name in
-/// `PhpType::Named(name.to_string())` at call sites that already have
+/// `PhpType::Named(atom(name.as_ref()))` at call sites that already have
 /// `&str` class names.
 pub(crate) fn is_subtype_of_names(
     subtype_name: &str,
@@ -272,8 +273,8 @@ pub(crate) fn is_subtype_of_names(
 ) -> bool {
     use crate::php_type::PhpType;
     is_subtype_of_typed(
-        &PhpType::Named(subtype_name.to_string()),
-        &PhpType::Named(supertype_name.to_string()),
+        &PhpType::Named(atom(subtype_name)),
+        &PhpType::Named(atom(supertype_name)),
         class_loader,
     )
 }
@@ -287,11 +288,7 @@ pub(crate) fn is_subtype_of_named(
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
 ) -> bool {
     use crate::php_type::PhpType;
-    is_subtype_of_typed(
-        subtype,
-        &PhpType::Named(supertype_name.to_string()),
-        class_loader,
-    )
+    is_subtype_of_typed(subtype, &PhpType::Named(atom(supertype_name)), class_loader)
 }
 
 /// Check whether `subtype` is a subtype of `supertype`, combining
@@ -437,7 +434,7 @@ pub(crate) fn is_subtype_of_typed(
     // name is a class-string of some object.
     if let (PhpType::ClassString(sub_inner), PhpType::ClassString(sup_inner)) = (subtype, supertype)
     {
-        let object_bound = PhpType::Named("object".to_string());
+        let object_bound = PhpType::Named(atom("object"));
         let sub = sub_inner.as_deref().unwrap_or(&object_bound);
         let sup = sup_inner.as_deref().unwrap_or(&object_bound);
         return is_subtype_of_typed(sub, sup, class_loader);

src/completion/phpdoc/generation/build.rs

@@ -2,6 +2,7 @@
 //! from a classified declaration, plus the type-enrichment helpers
 //! that decide when a native type hint needs a PHPDoc tag at all.
 
+use crate::atom::atom;
 use std::collections::HashMap;
 use std::sync::Arc;
 
@@ -209,7 +210,7 @@ pub(crate) fn enrichment_plain_typed(
         let args: Vec<PhpType> = cls
             .template_params
             .iter()
-            .map(|s| PhpType::Named(s.to_string()))
+            .map(|s| PhpType::Named(atom(s.as_ref())))
             .collect();
         return Some(PhpType::Generic(name.to_string(), args));
     }

src/completion/source/helpers.rs

@@ -20,6 +20,7 @@
 /// All functions in this module are free functions (not methods on
 /// `Backend`).  Cross-module dependencies that previously used `Self::`
 /// are called via their canonical module paths.
+use crate::atom::atom;
 use std::sync::Arc;
 
 use crate::php_type::PhpType;
@@ -508,8 +509,8 @@ fn infer_type_from_simple_expr(expr: &str) -> Option<PhpType> {
             "int" | "integer" => return Some(PhpType::int()),
             "float" | "double" | "real" => return Some(PhpType::float()),
             "bool" | "boolean" => return Some(PhpType::bool()),
-            "array" => return Some(PhpType::Named("array".to_string())),
-            "object" => return Some(PhpType::Named("object".to_string())),
+            "array" => return Some(PhpType::Named(atom("array"))),
+            "object" => return Some(PhpType::Named(atom("object"))),
             _ => {} // might be a parenthesized expression, not a cast
         }
     }
@@ -722,13 +723,10 @@ pub(crate) fn resolve_first_class_callable_return_type(
                 if classes.is_empty() {
                     None
                 } else if classes.len() == 1 {
-                    Some(PhpType::Named(classes[0].fqn().to_string()))
+                    Some(PhpType::Named(classes[0].fqn()))
                 } else {
                     Some(PhpType::Union(
-                        classes
-                            .iter()
-                            .map(|c| PhpType::Named(c.fqn().to_string()))
-                            .collect(),
+                        classes.iter().map(|c| PhpType::Named(c.fqn())).collect(),
                     ))
                 }
             })

src/completion/source/throws_analysis/cross_file.rs

@@ -15,6 +15,7 @@
 //! - `new ClassName(…)` — the class is loaded and the constructor's
 //!   `@throws` tags are propagated.
 
+use crate::atom::atom;
 use std::collections::HashMap;
 use std::sync::Arc;
 
@@ -194,7 +195,7 @@ pub(crate) fn find_uncaught_throw_types_with_context(
             } else {
                 resolved
             };
-            Some(PhpType::Named(fqn))
+            Some(PhpType::Named(atom(&fqn)))
         }
     }
 
@@ -443,15 +444,12 @@ pub(crate) fn parse_param_type_map(signature: &str) -> Vec<(String, PhpType)> {
         let parsed = PhpType::parse(type_token);
         let non_null = parsed.non_null_type().unwrap_or_else(|| parsed.clone());
         if let Some(name) = non_null.base_name() {
-            result.push((var_name.to_string(), PhpType::Named(name.to_string())));
+            result.push((var_name.to_string(), PhpType::Named(atom(name))));
         } else {
             // Fallback for scalars and other non-class types.
             let cleaned_type = type_token.trim_start_matches('?').trim_start_matches('\\');
             if !cleaned_type.is_empty() {
-                result.push((
-                    var_name.to_string(),
-                    PhpType::Named(cleaned_type.to_string()),
-                ));
+                result.push((var_name.to_string(), PhpType::Named(atom(cleaned_type))));
             }
         }
     }
@@ -482,11 +480,11 @@ pub(crate) fn find_cross_file_propagated_throws(
         .into_iter()
         .map(|(name, ty)| {
             let resolved = if let Some(base) = ty.base_name() {
-                PhpType::Named(crate::util::resolve_to_fqn(
+                PhpType::Named(atom(&crate::util::resolve_to_fqn(
                     base,
                     ctx.use_map,
                     ctx.file_namespace,
-                ))
+                )))
             } else {
                 ty
             };

src/completion/source/throws_analysis/scanning.rs

@@ -10,6 +10,7 @@
 //!   - Look up a method's `@throws` tags from its docblock
 //!   - Extract the function body following a docblock
 
+use crate::atom::atom;
 use crate::completion::source::comment_position::position_to_byte_offset;
 use crate::php_type::PhpType;
 use crate::text_scan::{
@@ -80,7 +81,7 @@ pub(crate) fn find_throw_statements(body: &str) -> Vec<ThrowInfo> {
                     let type_name = &after_new[..type_end];
                     if !type_name.is_empty() {
                         results.push(ThrowInfo {
-                            type_name: PhpType::Named(type_name.to_string()),
+                            type_name: PhpType::Named(atom(type_name)),
                             offset: pos,
                         });
                     }
@@ -298,7 +299,7 @@ pub(crate) fn find_inline_throws_annotations(body: &str) -> Vec<ThrowInfo> {
                                 .trim_end_matches('/');
                             if !clean.is_empty() && !clean.starts_with('$') {
                                 results.push(ThrowInfo {
-                                    type_name: PhpType::Named(clean.to_string()),
+                                    type_name: PhpType::Named(atom(clean)),
                                     offset: doc_start,
                                 });
                             }
@@ -357,7 +358,7 @@ pub(crate) fn find_method_return_type(file_content: &str, method_name: &str) ->
                     let parsed = PhpType::parse(type_str);
                     let non_null = parsed.non_null_type().unwrap_or_else(|| parsed.clone());
                     if let Some(name) = non_null.base_name() {
-                        return Some(PhpType::Named(name.to_string()));
+                        return Some(PhpType::Named(atom(name)));
                     }
                 }
             }
@@ -381,7 +382,7 @@ pub(crate) fn find_method_return_type(file_content: &str, method_name: &str) ->
                             && !parsed.is_mixed()
                             && !parsed.is_self_like()
                         {
-                            return Some(PhpType::Named(name.to_string()));
+                            return Some(PhpType::Named(atom(name)));
                         }
                     }
                 }
@@ -434,7 +435,7 @@ pub(crate) fn find_method_throws_tags(file_content: &str, method_name: &str) ->
                             .trim_end_matches('*')
                             .trim_start_matches('\\');
                         if !clean.is_empty() {
-                            throws.push(PhpType::Named(clean.to_string()));
+                            throws.push(PhpType::Named(atom(clean)));
                         }
                     }
                 }