From f82bb675556097c5d99a62ad6b3b4c19023a96ae Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 19 Jul 2024 07:13:51 -0700 Subject: [PATCH] [red-knot] trace file when inferring types (#12401) When poring over traces, the ones that just include a definition or symbol or expression ID aren't very useful, because you don't know which file it comes from. This adds that information to the trace. I guess the downside here is that if calling `.file(db)` on a scope/definition/expression would execute other traced code, it would be marked as outside the span? I don't think that's a concern, because I don't think a simple field access on a tracked struct should ever execute our code. If I'm wrong and this is a problem, it seems like the tracing crate has this feature where you can record a field as `tracing::field::Empty` and then fill in its value later with `span.record(...)`, but when I tried this it wasn't working for me, not sure why. I think there's a lot more we can do to make our tracing output more useful for debugging (e.g. record an event whenever a definition/symbol/expression/use id is created with the details of that definition/symbol/expression/use), this is just dipping my toes in the water. --- crates/red_knot_python_semantic/src/types/infer.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 73178be1b0b9b..75e7a34f13cca 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -46,9 +46,9 @@ use crate::Db; /// scope. #[salsa::tracked(return_ref)] pub(crate) fn infer_scope_types<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> TypeInference<'db> { - let _span = tracing::trace_span!("infer_scope_types", ?scope).entered(); - let file = scope.file(db); + let _span = tracing::trace_span!("infer_scope_types", ?scope, ?file).entered(); + // Using the index here is fine because the code below depends on the AST anyway. // The isolation of the query is by the return inferred types. let index = semantic_index(db, file); @@ -63,9 +63,10 @@ pub(crate) fn infer_definition_types<'db>( db: &'db dyn Db, definition: Definition<'db>, ) -> TypeInference<'db> { - let _span = tracing::trace_span!("infer_definition_types", ?definition).entered(); + let file = definition.file(db); + let _span = tracing::trace_span!("infer_definition_types", ?definition, ?file,).entered(); - let index = semantic_index(db, definition.file(db)); + let index = semantic_index(db, file); TypeInferenceBuilder::new(db, InferenceRegion::Definition(definition), index).finish() } @@ -80,9 +81,10 @@ pub(crate) fn infer_expression_types<'db>( db: &'db dyn Db, expression: Expression<'db>, ) -> TypeInference<'db> { - let _span = tracing::trace_span!("infer_expression_types", ?expression).entered(); + let file = expression.file(db); + let _span = tracing::trace_span!("infer_expression_types", ?expression, ?file).entered(); - let index = semantic_index(db, expression.file(db)); + let index = semantic_index(db, file); TypeInferenceBuilder::new(db, InferenceRegion::Expression(expression), index).finish() }