SymbolKind stores owned strings per span

Author: AJenbo

docs/todo.md

@@ -27,9 +27,9 @@ within the same impact tier.
 | --- | --------------------------------------------------------------------------------------------------------------------- | ---------- | ------ |
 | P36 | [Diagnostic-path call sites re-merge inheritance per call](todo/performance.md#p36-diagnostic-path-call-sites-re-merge-inheritance-per-call-instead-of-reading-the-resolved-class-cache)          | Medium      | Low         |
 | P38 | [Resolved type values are duplicated ~34x](todo/performance.md#p38-resolved-type-values-are-duplicated-34x)                                                          | High        | High        |
-| P39 | [`SymbolKind` stores owned strings per span](todo/performance.md#p39-symbolkind-stores-owned-strings-per-span)                                                       | Medium      | Low-Medium  |
 | P40 | [`method_index` is a per-class `HashMap` even when the member vec is shared](todo/performance.md#p40-method_index-is-a-per-class-hashmap-even-when-the-member-vec-is-shared) | Low-Medium  | Low         |
 | P41 | [3.8 M live allocations cost ~170 MB in allocator overhead](todo/performance.md#p41-38-m-live-allocations-cost-170-mb-in-allocator-overhead)                          | Medium      | Medium      |
+| P43 | [`MemberAccess.subject_text` still allocates a `String` per span](todo/performance.md#p43-memberaccesssubject_text-still-allocates-a-string-per-span) | Low-Medium  | Medium      |
 | P33 | [Workspace diagnostics leaves the whole project fully resolved in memory](todo/performance.md#p33-workspace-diagnostics-leaves-the-whole-project-fully-resolved-in-memory) | High        | Medium-High |
 | X10 | [Interactive requests block on the workspace index lock during initial indexing](todo/indexing.md#x10-interactive-requests-block-on-the-workspace-index-lock-during-initial-indexing) | Medium      | Medium      |
 | L21 | [Tighten the supertype-where-subtype comparison escape hatch (blocked on resolver precision)](todo/laravel.md#l21-tighten-the-supertype-where-subtype-comparison-escape-hatch-blocked-on-resolver-precision) | Medium      | High        |

docs/todo/performance.md

@@ -1156,38 +1156,6 @@ should keep owned payloads or use a separate bounded cache.
 
 ---
 
-## P39. `SymbolKind` stores owned strings per span
-
-**Impact: Medium · Effort: Low-Medium**
-
-`symbol_maps` is the second-largest store: 40 MB / 367 K allocations on
-the mid-size app, 64 MB / 602 K on the large one. The span vectors
-dominate it — 353 K spans costing 31 MB of struct bytes (88 B per
-`SymbolSpan`) plus 4.5 MB of string payloads across 254 K separate
-allocations.
-
-`SymbolSpan` is `{ start: u32, end: u32, kind: SymbolKind }`, and
-`SymbolKind` is fat because nearly every variant owns a `String`.
-`MemberAccess` owns two (`subject_text` and `member_name`).
-
-Two independent changes:
-
-1. **Interned names.** `member_name`, class names, variable names,
-   function names, and constant names are all drawn from bounded sets
-   already interned elsewhere. Making them `Atom` drops 16 B per field
-   and removes the per-span allocation entirely.
-2. **`subject_text` as a byte range.** It is the source text of the LHS
-   expression, which is unbounded free text and must not be interned.
-   Every consumer has the file content available, so storing
-   `(start, end)` offsets and slicing on demand removes the string
-   without losing anything.
-
-Together these should take `SymbolKind` from ~80 B to ~24-32 B,
-reclaiming roughly 17 MB on the large app and eliminating ~250 K
-allocations.
-
----
-
 ## P40. `method_index` is a per-class `HashMap` even when the member vec is shared
 
 **Impact: Low-Medium · Effort: Low**
@@ -1275,6 +1243,62 @@ headless mode rather than special-casing the `analyze` call site.
 
 ---
 
+## P43. `MemberAccess.subject_text` still allocates a `String` per span
+
+**Impact: Low-Medium · Effort: Medium**
+
+Every other name field on `SymbolKind` (`member_name`, class names,
+variable names, function names, constant names) is now `Atom`,
+eliminating their per-span allocation. `MemberAccess::subject_text` is
+the one field left as an owned `String`, and it is not a simple
+byte-range problem the way it first looked.
+
+`subject_text` is not a slice of the file's raw source — it is the
+*synthesized* canonical text `expr_to_subject_text` builds from the AST
+(`symbol_map/extraction/subject_text.rs`, `type_engine/subject_expr.rs`).
+It diverges from the raw bytes in ways consumers actually depend on:
+
+- `(new Foo($x))->bar()` lowers straight to `"Foo"`, dropping `new` and
+  the constructor arguments entirely — documented as intentional,
+  matching historical behaviour ("the constructed instance resolves
+  through its class").
+- Whitespace around `->`/`::` is normalised away (`$this -> foo` still
+  serialises to `"$this->foo"`); a raw source slice would keep the
+  spaces and break the byte-exact splitting `SubjectExpr::parse` does
+  (`split_last_arrow_raw` and friends have no whitespace tolerance).
+- Expressions the lowering can't represent collapse to `""`, and
+  several consumers key off `.is_empty()` as an explicit
+  "unresolvable" sentinel (e.g. `resolve_target_classes` callers). A
+  raw slice would hand them real, non-empty text instead and defeat
+  that check.
+
+Storing `(start, end)` into the file content and slicing at request
+time would reproduce none of this and would silently change subject
+resolution for the shapes above — exactly the class of silently-wrong
+result the project's no-diagnostic-suppression rule exists to prevent
+for diagnostics, and the same logic applies to any other resolution
+path.
+
+A safe version of the same idea is still possible, just bigger than a
+plain byte range:
+
+1. Add an enum (`SubjectText::Range { start, end } |
+   SubjectText::Owned(Box<str>)`), chosen at extraction time by
+   comparing the synthesized text against the raw byte range of the
+   object expression and only paying for `Owned` when they differ
+   (nested calls, `new`-as-subject, unsupported forms).
+2. Thread the file content through the ~15 call sites across `hover`,
+   `definition`, `references`, `diagnostics`, `code_actions`, and the
+   `symbol_map` test helpers that currently read `subject_text` as a
+   bare `&str`, since `SubjectText::as_str` needs it to resolve the
+   `Range` case.
+
+Worth re-measuring after: most `MemberAccess` subjects in typical PHP
+code are plain variables/`$this`/class names, which take the free
+`Range` path, so the `Owned` fallback should be rare in practice.
+
+---
+
 # Remaining anti-pattern fixes
 
 Most remaining depth-cap issues were addressed by eager class

src/definition/resolve.rs

@@ -357,11 +357,11 @@ impl Backend {
                 let ctx = self.file_context(uri);
                 let fqn = ctx.resolve_name_at(name, cursor_offset);
                 let mut candidates = vec![fqn];
-                if name.contains('\\') && !candidates.contains(name) {
-                    candidates.push(name.clone());
+                if name.contains('\\') && !candidates.iter().any(|c| c == name) {
+                    candidates.push(name.to_string());
                 }
-                if !candidates.contains(name) {
-                    candidates.push(name.clone());
+                if !candidates.iter().any(|c| c == name) {
+                    candidates.push(name.to_string());
                 }
                 self.resolve_function_definition(&candidates)
                     .map(|loc| vec![loc])
@@ -371,8 +371,8 @@ impl Backend {
                 let ctx = self.file_context(uri);
                 let fqn = ctx.resolve_name_at(name, cursor_offset);
                 let mut candidates = vec![fqn];
-                if !candidates.contains(name) {
-                    candidates.push(name.clone());
+                if !candidates.iter().any(|c| c == name) {
+                    candidates.push(name.to_string());
                 }
                 // Try class constant (Name::CONST) first — but the symbol
                 // map records class constants as MemberAccess, so this path

src/diagnostics/unknown_functions.rs

@@ -107,7 +107,7 @@ impl Backend {
                     name,
                     is_definition: true,
                 } => {
-                    let mut names = vec![name.clone()];
+                    let mut names = vec![name.to_string()];
                     if let Some(ns) = file_namespace {
                         names.push(format!("{}\\{}", ns, name));
                     }

src/highlight/mod.rs

@@ -61,7 +61,7 @@ impl Backend {
             SymbolKind::ClassReference { name, is_fqn, .. } => {
                 let ctx = self.file_context(uri);
                 let fqn = if *is_fqn {
-                    name.clone()
+                    name.to_string()
                 } else {
                     ctx.resolve_name_at(name, span.start)
                 };
@@ -224,7 +224,7 @@ impl Backend {
             let fqn = match &span.kind {
                 SymbolKind::ClassReference { name, is_fqn, .. } => {
                     if *is_fqn {
-                        name.clone()
+                        name.to_string()
                     } else {
                         Self::resolve_to_fqn(name, use_map, namespace)
                     }

src/reference_index.rs

@@ -652,7 +652,7 @@ mod tests {
                 start: 0,
                 end: name.len() as u32,
                 kind: SymbolKind::ClassDeclaration {
-                    name: name.to_string(),
+                    name: crate::atom::atom(name),
                 },
             }],
             ..SymbolMap::default()

src/references/classes.rs

@@ -94,7 +94,7 @@ impl Backend {
                 let matched = match &span.kind {
                     SymbolKind::ClassReference { name, is_fqn, .. } => {
                         let resolved = if *is_fqn {
-                            name.clone()
+                            name.to_string()
                         } else if let Some(fqn) =
                             resolved_names.as_ref().and_then(|rn| rn.get(span.start))
                         {

src/references/dispatch.rs

@@ -180,7 +180,7 @@ impl Backend {
             SymbolKind::ClassReference { name, is_fqn, .. } => {
                 let ctx = self.file_context(uri);
                 let fqn = if *is_fqn {
-                    name.clone()
+                    name.to_string()
                 } else {
                     ctx.resolve_name_at(name, span_start)
                 };

src/rename/class.rs

@@ -32,7 +32,7 @@ impl Backend {
             SymbolKind::ClassReference { name, is_fqn, .. } => {
                 let ctx = self.file_context(uri);
                 let fqn = if *is_fqn {
-                    name.clone()
+                    name.to_string()
                 } else {
                     ctx.resolve_name_at(name, offset)
                 };

src/rename/prepare.rs

@@ -226,14 +226,14 @@ impl Backend {
                 // Include the `$` prefix in the range — the span already does.
                 Some((format!("${}", name), range))
             }
-            SymbolKind::CompactVariable { name } => Some((name.clone(), range)),
-            SymbolKind::ClassReference { name, .. } => Some((name.clone(), range)),
-            SymbolKind::ClassDeclaration { name } => Some((name.clone(), range)),
-            SymbolKind::MemberAccess { member_name, .. } => Some((member_name.clone(), range)),
-            SymbolKind::MemberDeclaration { name, .. } => Some((name.clone(), range)),
-            SymbolKind::FunctionCall { name, .. } => Some((name.clone(), range)),
-            SymbolKind::ConstantReference { name } => Some((name.clone(), range)),
-            SymbolKind::NamespaceDeclaration { name } => Some((name.clone(), range)),
+            SymbolKind::CompactVariable { name } => Some((name.to_string(), range)),
+            SymbolKind::ClassReference { name, .. } => Some((name.to_string(), range)),
+            SymbolKind::ClassDeclaration { name } => Some((name.to_string(), range)),
+            SymbolKind::MemberAccess { member_name, .. } => Some((member_name.to_string(), range)),
+            SymbolKind::MemberDeclaration { name, .. } => Some((name.to_string(), range)),
+            SymbolKind::FunctionCall { name, .. } => Some((name.to_string(), range)),
+            SymbolKind::ConstantReference { name } => Some((name.to_string(), range)),
+            SymbolKind::NamespaceDeclaration { name } => Some((name.to_string(), range)),
             SymbolKind::LaravelMacroString { name } => Some((name.clone(), range)),
             SymbolKind::SelfStaticParent { .. } => None,
             SymbolKind::LaravelStringKey { .. }