Skip to content

Commit

Permalink
[red-knot] trace file when inferring types (#12401)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
carljm committed Jul 19, 2024
1 parent 5f96f69 commit f82bb67
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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()
}
Expand All @@ -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()
}
Expand Down

0 comments on commit f82bb67

Please sign in to comment.