[ty] Have SemanticIndex::place_table() and SemanticIndex::use_def_map return references

Author: AlexWaygood

crates/ty_python_semantic/src/semantic_index.rs

@@ -70,8 +70,7 @@ pub(crate) fn place_table<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> Arc<Plac
     let file = scope.file(db);
     let _span = tracing::trace_span!("place_table", scope=?scope.as_id(), ?file).entered();
     let index = semantic_index(db, file);
-
-    index.place_table(scope.file_scope_id(db))
+    Arc::clone(index.place_table(scope.file_scope_id(db)))
 }
 
 /// Returns the set of modules that are imported anywhere in `file`.
@@ -100,8 +99,7 @@ pub(crate) fn use_def_map<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> Arc<UseD
     let file = scope.file(db);
     let _span = tracing::trace_span!("use_def_map", scope=?scope.as_id(), ?file).entered();
     let index = semantic_index(db, file);
-
-    index.use_def_map(scope.file_scope_id(db))
+    Arc::clone(index.use_def_map(scope.file_scope_id(db)))
 }
 
 /// Returns all attribute assignments (and their method scope IDs) with a symbol name matching
@@ -252,17 +250,17 @@ impl<'db> SemanticIndex<'db> {
     /// Use the Salsa cached [`place_table()`] query if you only need the
     /// place table for a single scope.
     #[track_caller]
-    pub(super) fn place_table(&self, scope_id: FileScopeId) -> Arc<PlaceTable> {
-        self.place_tables[scope_id].clone()
+    pub(super) fn place_table(&self, scope_id: FileScopeId) -> &Arc<PlaceTable> {
+        &self.place_tables[scope_id]
     }
 
     /// Returns the use-def map for a specific scope.
     ///
     /// Use the Salsa cached [`use_def_map()`] query if you only need the
     /// use-def map for a single scope.
     #[track_caller]
-    pub(super) fn use_def_map(&self, scope_id: FileScopeId) -> Arc<UseDefMap<'_>> {
-        self.use_def_maps[scope_id].clone()
+    pub(super) fn use_def_map(&self, scope_id: FileScopeId) -> &Arc<UseDefMap<'_>> {
+        &self.use_def_maps[scope_id]
     }
 
     #[track_caller]
@@ -907,7 +905,7 @@ y = 2
         );
 
         let class_table = index.place_table(class_scope_id);
-        assert_eq!(names(&class_table), vec!["x"]);
+        assert_eq!(names(class_table), vec!["x"]);
 
         let use_def = index.use_def_map(class_scope_id);
         let binding = use_def
@@ -929,7 +927,7 @@ y = 2
         let index = semantic_index(&db, file);
         let global_table = index.place_table(FileScopeId::global());
 
-        assert_eq!(names(&global_table), vec!["func", "y"]);
+        assert_eq!(names(global_table), vec!["func", "y"]);
 
         let [(function_scope_id, function_scope)] = index
             .child_scopes(FileScopeId::global())
@@ -944,7 +942,7 @@ y = 2
         );
 
         let function_table = index.place_table(function_scope_id);
-        assert_eq!(names(&function_table), vec!["x"]);
+        assert_eq!(names(function_table), vec!["x"]);
 
         let use_def = index.use_def_map(function_scope_id);
         let binding = use_def
@@ -976,7 +974,7 @@ def f(a: str, /, b: str, c: int = 1, *args, d: int = 2, **kwargs):
 
         let function_table = index.place_table(function_scope_id);
         assert_eq!(
-            names(&function_table),
+            names(function_table),
             vec!["a", "b", "c", "d", "args", "kwargs"],
         );
 
@@ -1021,7 +1019,7 @@ def f(a: str, /, b: str, c: int = 1, *args, d: int = 2, **kwargs):
 
         let lambda_table = index.place_table(lambda_scope_id);
         assert_eq!(
-            names(&lambda_table),
+            names(lambda_table),
             vec!["a", "b", "c", "d", "args", "kwargs"],
         );
 
@@ -1062,7 +1060,7 @@ def f(a: str, /, b: str, c: int = 1, *args, d: int = 2, **kwargs):
         let index = semantic_index(&db, file);
         let global_table = index.place_table(FileScopeId::global());
 
-        assert_eq!(names(&global_table), vec!["iter1"]);
+        assert_eq!(names(global_table), vec!["iter1"]);
 
         let [(comprehension_scope_id, comprehension_scope)] = index
             .child_scopes(FileScopeId::global())
@@ -1081,7 +1079,7 @@ def f(a: str, /, b: str, c: int = 1, *args, d: int = 2, **kwargs):
 
         let comprehension_symbol_table = index.place_table(comprehension_scope_id);
 
-        assert_eq!(names(&comprehension_symbol_table), vec!["x", "y"]);
+        assert_eq!(names(comprehension_symbol_table), vec!["x", "y"]);
 
         let use_def = index.use_def_map(comprehension_scope_id);
         for name in ["x", "y"] {
@@ -1159,7 +1157,7 @@ def f(a: str, /, b: str, c: int = 1, *args, d: int = 2, **kwargs):
         let index = semantic_index(&db, file);
         let global_table = index.place_table(FileScopeId::global());
 
-        assert_eq!(names(&global_table), vec!["iter1"]);
+        assert_eq!(names(global_table), vec!["iter1"]);
 
         let [(comprehension_scope_id, comprehension_scope)] = index
             .child_scopes(FileScopeId::global())
@@ -1178,7 +1176,7 @@ def f(a: str, /, b: str, c: int = 1, *args, d: int = 2, **kwargs):
 
         let comprehension_symbol_table = index.place_table(comprehension_scope_id);
 
-        assert_eq!(names(&comprehension_symbol_table), vec!["y", "iter2"]);
+        assert_eq!(names(comprehension_symbol_table), vec!["y", "iter2"]);
 
         let [(inner_comprehension_scope_id, inner_comprehension_scope)] = index
             .child_scopes(comprehension_scope_id)
@@ -1197,7 +1195,7 @@ def f(a: str, /, b: str, c: int = 1, *args, d: int = 2, **kwargs):
 
         let inner_comprehension_symbol_table = index.place_table(inner_comprehension_scope_id);
 
-        assert_eq!(names(&inner_comprehension_symbol_table), vec!["x"]);
+        assert_eq!(names(inner_comprehension_symbol_table), vec!["x"]);
     }
 
     #[test]
@@ -1212,7 +1210,7 @@ with item1 as x, item2 as y:
         let index = semantic_index(&db, file);
         let global_table = index.place_table(FileScopeId::global());
 
-        assert_eq!(names(&global_table), vec!["item1", "x", "item2", "y"]);
+        assert_eq!(names(global_table), vec!["item1", "x", "item2", "y"]);
 
         let use_def = index.use_def_map(FileScopeId::global());
         for name in ["x", "y"] {
@@ -1235,7 +1233,7 @@ with context() as (x, y):
         let index = semantic_index(&db, file);
         let global_table = index.place_table(FileScopeId::global());
 
-        assert_eq!(names(&global_table), vec!["context", "x", "y"]);
+        assert_eq!(names(global_table), vec!["context", "x", "y"]);
 
         let use_def = index.use_def_map(FileScopeId::global());
         for name in ["x", "y"] {
@@ -1260,7 +1258,7 @@ def func():
         let index = semantic_index(&db, file);
         let global_table = index.place_table(FileScopeId::global());
 
-        assert_eq!(names(&global_table), vec!["func"]);
+        assert_eq!(names(global_table), vec!["func"]);
         let [
             (func_scope1_id, func_scope_1),
             (func_scope2_id, func_scope_2),
@@ -1285,8 +1283,8 @@ def func():
 
         let func1_table = index.place_table(func_scope1_id);
         let func2_table = index.place_table(func_scope2_id);
-        assert_eq!(names(&func1_table), vec!["x"]);
-        assert_eq!(names(&func2_table), vec!["y"]);
+        assert_eq!(names(func1_table), vec!["x"]);
+        assert_eq!(names(func2_table), vec!["y"]);
 
         let use_def = index.use_def_map(FileScopeId::global());
         let binding = use_def
@@ -1308,7 +1306,7 @@ def func[T]():
         let index = semantic_index(&db, file);
         let global_table = index.place_table(FileScopeId::global());
 
-        assert_eq!(names(&global_table), vec!["func"]);
+        assert_eq!(names(global_table), vec!["func"]);
 
         let [(ann_scope_id, ann_scope)] = index
             .child_scopes(FileScopeId::global())
@@ -1323,7 +1321,7 @@ def func[T]():
             "func"
         );
         let ann_table = index.place_table(ann_scope_id);
-        assert_eq!(names(&ann_table), vec!["T"]);
+        assert_eq!(names(ann_table), vec!["T"]);
 
         let [(func_scope_id, func_scope)] =
             index.child_scopes(ann_scope_id).collect::<Vec<_>>()[..]
@@ -1336,7 +1334,7 @@ def func[T]():
             "func"
         );
         let func_table = index.place_table(func_scope_id);
-        assert_eq!(names(&func_table), vec!["x"]);
+        assert_eq!(names(func_table), vec!["x"]);
     }
 
     #[test]
@@ -1352,7 +1350,7 @@ class C[T]:
         let index = semantic_index(&db, file);
         let global_table = index.place_table(FileScopeId::global());
 
-        assert_eq!(names(&global_table), vec!["C"]);
+        assert_eq!(names(global_table), vec!["C"]);
 
         let [(ann_scope_id, ann_scope)] = index
             .child_scopes(FileScopeId::global())
@@ -1364,7 +1362,7 @@ class C[T]:
         assert_eq!(ann_scope.kind(), ScopeKind::TypeParams);
         assert_eq!(ann_scope_id.to_scope_id(&db, file).name(&db, &module), "C");
         let ann_table = index.place_table(ann_scope_id);
-        assert_eq!(names(&ann_table), vec!["T"]);
+        assert_eq!(names(ann_table), vec!["T"]);
         assert!(
             ann_table
                 .symbol_by_name("T")
@@ -1383,7 +1381,7 @@ class C[T]:
             class_scope_id.to_scope_id(&db, file).name(&db, &module),
             "C"
         );
-        assert_eq!(names(&index.place_table(class_scope_id)), vec!["x"]);
+        assert_eq!(names(index.place_table(class_scope_id)), vec!["x"]);
     }
 
     #[test]

crates/ty_python_semantic/src/types/ide_support.rs

@@ -306,8 +306,7 @@ impl<'db> AllMembers<'db> {
             let file = class_body_scope.file(db);
             let index = semantic_index(db, file);
             for function_scope_id in attribute_scopes(db, class_body_scope) {
-                let place_table = index.place_table(function_scope_id);
-                for place_expr in place_table.members() {
+                for place_expr in index.place_table(function_scope_id).members() {
                     let Some(name) = place_expr.as_instance_attribute() else {
                         continue;
                     };
@@ -410,8 +409,9 @@ pub fn definition_kind_for_name<'db>(
     let symbol_id = place_table.symbol_id(name_str)?;
 
     // Get the use-def map and look up definitions for this place
-    let use_def_map = index.use_def_map(file_scope);
-    let declarations = use_def_map.all_reachable_symbol_declarations(symbol_id);
+    let declarations = index
+        .use_def_map(file_scope)
+        .all_reachable_symbol_declarations(symbol_id);
 
     // Find the first valid definition and return its kind
     for declaration in declarations {
@@ -661,9 +661,10 @@ pub fn definitions_for_attribute<'db>(
             let index = semantic_index(db, file);
 
             for function_scope_id in attribute_scopes(db, class_scope) {
-                let place_table = index.place_table(function_scope_id);
-
-                if let Some(place_id) = place_table.member_id_by_instance_attribute_name(name_str) {
+                if let Some(place_id) = index
+                    .place_table(function_scope_id)
+                    .member_id_by_instance_attribute_name(name_str)
+                {
                     let use_def = index.use_def_map(function_scope_id);
 
                     // Check declarations first

crates/ty_python_semantic/src/types/infer.rs

@@ -1869,7 +1869,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         let mut bound_ty = ty;
 
         let global_use_def_map = self.index.use_def_map(FileScopeId::global());
-        let nonlocal_use_def_map;
         let place_id = binding.place(self.db());
         let place = place_table.place(place_id);
 
@@ -1929,9 +1928,10 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                     }
                     // We found the closest definition. Note that (as in `infer_place_load`) this does
                     // *not* need to be a binding. It could be just a declaration, e.g. `x: int`.
-                    nonlocal_use_def_map = self.index.use_def_map(enclosing_scope_file_id);
-                    declarations =
-                        nonlocal_use_def_map.end_of_scope_symbol_declarations(enclosing_symbol_id);
+                    declarations = self
+                        .index
+                        .use_def_map(enclosing_scope_file_id)
+                        .end_of_scope_symbol_declarations(enclosing_symbol_id);
                     is_local = false;
                     break;
                 }
@@ -2128,8 +2128,10 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             .or_fall_back_to(self.db(), || {
                 // Fallback to bindings declared on `types.ModuleType` if it's a global symbol
                 let scope = self.scope().file_scope_id(self.db());
-                let place_table = self.index.place_table(scope);
-                let place = place_table.place(declaration.place(self.db()));
+                let place = self
+                    .index
+                    .place_table(scope)
+                    .place(declaration.place(self.db()));
                 if let PlaceExprRef::Symbol(symbol) = &place {
                     if scope.is_global() {
                         module_type_implicit_global_symbol(self.db(), symbol.name())
@@ -2522,8 +2524,10 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                     invalid.ty,
                 );
             }
-            let use_def = self.index.use_def_map(scope_id);
-            if use_def.can_implicitly_return_none(self.db())
+            if self
+                .index
+                .use_def_map(scope_id)
+                .can_implicitly_return_none(self.db())
                 && !Type::none(self.db()).is_assignable_to(self.db(), expected_ty)
             {
                 let no_return = self.return_types_and_ranges.is_empty();
@@ -5189,20 +5193,11 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
 
         let module_ty = Type::module_literal(self.db(), self.file(), module);
 
-        // The indirection of having `star_import_info` as a separate variable
-        // is required in order to make the borrow checker happy.
-        let star_import_info = definition
-            .kind(self.db())
-            .as_star_import()
-            .map(|star_import| {
-                let place_table = self
-                    .index
-                    .place_table(self.scope().file_scope_id(self.db()));
-                (star_import, place_table)
-            });
-
-        let name = if let Some((star_import, symbol_table)) = star_import_info.as_ref() {
-            symbol_table.symbol(star_import.symbol_id()).name()
+        let name = if let Some(star_import) = definition.kind(self.db()).as_star_import() {
+            self.index
+                .place_table(self.scope().file_scope_id(self.db()))
+                .symbol(star_import.symbol_id())
+                .name()
         } else {
             &alias.name.id
         };