Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot] Internal refactoring of visibility constraints API #15913

Merged
merged 6 commits into from
Feb 3, 2025

Conversation

dcreager
Copy link
Member

@dcreager dcreager commented Feb 3, 2025

This extracts some pure refactoring noise from #15861. This changes the API for creating and evaluating visibility constraints, but does not change how they are respresented internally. There should be no behavioral or performance changes in this PR.

Changes:

  • Hide the internal representation isn't changed, so that we can make changes to it in [red-knot] Use ternary decision diagrams (TDDs) for visibility constraints #15861.
  • Add a separate builder type for visibility constraints. (With TDDs, we will have some additional builder state that we can throw away once we're done constructing.)
  • Remove a layer of helper methods from UseDefMapBuilder, making SemanticIndexBuilder responsible for constructing whatever visibility constraints it needs.

@dcreager dcreager changed the title Dcreager/vc api [red-knot] Internal refactoring of visibility constraints API Feb 3, 2025
@dcreager dcreager added internal An internal refactor or improvement red-knot Multi-file analysis & type inference labels Feb 3, 2025
pub(crate) struct VisibilityConstraint<'db>(VisibilityConstraintInner<'db>);

#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) enum VisibilityConstraintInner<'db> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this as a separate type made it easier to verify that no callers are constructing this manually, and always use one of the builder's constructor methods

Comment on lines +199 to +201
/// A newtype-index for a visibility constraint in a particular scope.
#[newtype_index]
pub(crate) struct ScopedVisibilityConstraintId;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into this module to be near the other visibility constraint stuff

@dcreager dcreager merged commit 0529ad6 into main Feb 3, 2025
21 checks passed
@dcreager dcreager deleted the dcreager/vc-api branch February 3, 2025 20:13
dcreager added a commit that referenced this pull request Feb 4, 2025
* main: (66 commits)
  [red-knot] Use ternary decision diagrams (TDDs) for visibility constraints (#15861)
  [`pyupgrade`] Rename private type parameters in PEP 695 generics (`UP049`) (#15862)
  Simplify the `StringFlags` trait (#15944)
  [`flake8-pyi`] Make `PYI019` autofixable for `.py` files in preview mode as well as stubs (#15889)
  Docs (`linter.md`): clarify that Python files are always searched for in subdirectories (#15882)
  [`flake8-pyi`] Make PEP-695 functions with multiple type parameters fixable by PYI019 again (#15938)
  [red-knot] Use unambiguous invalid-syntax-construct for suppression comment test (#15933)
  Make `Binding::range()` point to the range of a type parameter's name, not the full type parameter (#15935)
  Update black deviations (#15928)
  [red-knot] MDTest: Fix line numbers in error messages (#15932)
  Preserve triple quotes and prefixes for strings (#15818)
  [red-knot] Hand-written MDTest parser (#15926)
  [`pylint`] Fix missing parens in unsafe fix for `unnecessary-dunder-call` (`PLC2801`) (#15762)
  nit: docs for ignore & select (#15883)
  [airflow] `BashOperator` has been moved to `airflow.providers.standard.operators.bash.BashOperator` (AIR302) (#15922)
  [`flake8-logging`] `.exception()` and `exc_info=` outside exception handlers (`LOG004`, `LOG014`) (#15799)
  [red-knot] Enforce specifying paths for mdtest code blocks in a separate preceding line (#15890)
  [red-knot] Internal refactoring of visibility constraints API (#15913)
  [red-knot] Implicit instance attributes (#15811)
  [`flake8-comprehensions`] Handle extraneous parentheses around list comprehension (`C403`) (#15877)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement red-knot Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants