Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
carljm committed Jul 16, 2024
1 parent 75791b6 commit e92d796
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 111 deletions.
25 changes: 1 addition & 24 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ strum_macros = { version = "0.26.0" }
syn = { version = "2.0.55" }
tempfile = { version = "3.9.0" }
test-case = { version = "3.3.1" }
textwrap = { version = "0.16.1" }
thiserror = { version = "1.0.58" }
tikv-jemallocator = { version = "0.6.0" }
toml = { version = "0.8.11" }
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ red_knot_module_resolver = { workspace = true }
ruff_db = { workspace = true }
ruff_index = { workspace = true }
ruff_python_ast = { workspace = true }
ruff_python_trivia = { workspace = true }
ruff_text_size = { workspace = true }

bitflags = { workspace = true }
Expand All @@ -27,7 +28,6 @@ hashbrown = { workspace = true }
[dev-dependencies]
anyhow = { workspace = true }
ruff_python_parser = { workspace = true }
textwrap = { workspace = true }

[lints]
workspace = true
Expand Down
7 changes: 7 additions & 0 deletions crates/red_knot_python_semantic/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub(crate) mod tests {
use ruff_db::system::{DbWithTestSystem, System, TestSystem};
use ruff_db::vendored::VendoredFileSystem;
use ruff_db::{Db as SourceDb, Jar as SourceJar, Upcast};
use ruff_python_trivia::textwrap;

use super::{Db, Jar};

Expand Down Expand Up @@ -88,6 +89,12 @@ pub(crate) mod tests {
pub(crate) fn clear_salsa_events(&mut self) {
self.take_salsa_events();
}

/// Write auto-dedented text to a file.
pub(crate) fn write_dedented(&mut self, path: &str, content: &str) -> anyhow::Result<()> {
self.write_file(path, textwrap::dedent(content))?;
Ok(())
}
}

impl DbWithTestSystem for TestDb {
Expand Down
7 changes: 5 additions & 2 deletions crates/red_knot_python_semantic/src/semantic_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ use crate::semantic_index::expression::Expression;
use crate::semantic_index::symbol::{
FileScopeId, NodeWithScopeKey, NodeWithScopeRef, Scope, ScopeId, ScopedSymbolId, SymbolTable,
};
use crate::semantic_index::usedef::UseDefMap;
use crate::semantic_index::use_def::UseDefMap;
use crate::Db;

pub mod ast_ids;
mod builder;
pub mod definition;
pub mod expression;
pub mod symbol;
pub mod usedef;
pub mod use_def;

type SymbolMap = hashbrown::HashMap<ScopedSymbolId, (), ()>;

Expand Down Expand Up @@ -188,6 +188,9 @@ impl<'db> SemanticIndex<'db> {
}

/// Returns the [`Expression`] ingredient for an expression node.
/// Panics if we have no expression ingredient for that node. We can only call this method for
/// standalone-inferable expressions, which we call `add_standalone_expression` for in
/// [`SemanticIndexBuilder`].
pub(crate) fn expression(
&self,
expression_key: impl Into<ExpressionNodeKey>,
Expand Down
5 changes: 3 additions & 2 deletions crates/red_knot_python_semantic/src/semantic_index/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::semantic_index::symbol::{
FileScopeId, NodeWithScopeKey, NodeWithScopeRef, Scope, ScopeId, ScopedSymbolId, SymbolFlags,
SymbolTableBuilder,
};
use crate::semantic_index::usedef::{FlowSnapshot, UseDefMapBuilder};
use crate::semantic_index::use_def::{FlowSnapshot, UseDefMapBuilder};
use crate::semantic_index::SemanticIndex;
use crate::Db;

Expand Down Expand Up @@ -176,7 +176,8 @@ impl<'db> SemanticIndexBuilder<'db> {

self.definitions_by_node
.insert(definition_node.key(), definition);
self.current_use_def_map().record_def(symbol, definition);
self.current_use_def_map()
.record_definition(symbol, definition);

definition
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ struct Definitions {
impl Default for Definitions {
fn default() -> Self {
Self {
definitions: 0..0,
definitions: Range::default(),
may_be_unbound: true,
}
}
Expand All @@ -60,7 +60,7 @@ pub(super) struct UseDefMapBuilder<'db> {

definitions_by_use: IndexVec<ScopedUseId, Definitions>,

// builder state: currently visible definitions for each symbol
/// builder state: currently visible definitions for each symbol
definitions_by_symbol: IndexVec<ScopedSymbolId, Definitions>,
}

Expand All @@ -78,7 +78,11 @@ impl<'db> UseDefMapBuilder<'db> {
debug_assert_eq!(symbol, new_symbol);
}

pub(super) fn record_def(&mut self, symbol: ScopedSymbolId, definition: Definition<'db>) {
pub(super) fn record_definition(
&mut self,
symbol: ScopedSymbolId,
definition: Definition<'db>,
) {
let def_idx = self.all_definitions.len();
self.all_definitions.push(definition);
self.definitions_by_symbol[symbol] = Definitions {
Expand Down Expand Up @@ -110,7 +114,7 @@ impl<'db> UseDefMapBuilder<'db> {

pub(super) fn merge(&mut self, state: &FlowSnapshot) {
for (symbol_id, to_merge) in state.definitions_by_symbol.iter_enumerated() {
let current = self.definitions_by_symbol.get_mut(symbol_id).unwrap();
let current = &mut self.definitions_by_symbol[symbol_id];
// if the symbol can be unbound in either predecessor, it can be unbound
current.may_be_unbound |= to_merge.may_be_unbound;
// merge the definition ranges
Expand Down
Loading

0 comments on commit e92d796

Please sign in to comment.