[red-knot] Rename and rework the CoreStdlibModule enum
#15071
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlexWaygood
requested review from
carljm,
MichaReiser and
sharkdp
as code owners
AlexWaygood
commented
Dec 19, 2024
Comment on lines
2669
to
2754
| pub fn try_from_file_and_symbol( | ||
| db: &'db dyn Db, | ||
| file: File, | ||
| instance_name: &str, | ||
| ) -> Option<Self> { | ||
| let candidate = match instance_name { | ||
| "Any" => Self::Any, | ||
| "ClassVar" => Self::ClassVar, | ||
| "Deque" => Self::Deque, | ||
| "List" => Self::List, | ||
| "Dict" => Self::Dict, | ||
| "DefaultDict" => Self::DefaultDict, | ||
| "Set" => Self::Set, | ||
| "FrozenSet" => Self::FrozenSet, | ||
| "Counter" => Self::Counter, | ||
| "ChainMap" => Self::ChainMap, | ||
| "OrderedDict" => Self::OrderedDict, | ||
| "Optional" => Self::Optional, | ||
| "Union" => Self::Union, | ||
| "NoReturn" => Self::NoReturn, | ||
| "Tuple" => Self::Tuple, | ||
| "Type" => Self::Type, | ||
| "Callable" => Self::Callable, | ||
| "Annotated" => Self::Annotated, | ||
| "Literal" => Self::Literal, | ||
| "Never" => Self::Never, | ||
| "Self" => Self::TypingSelf, | ||
| "Final" => Self::Final, | ||
| "Unpack" => Self::Unpack, | ||
| "Required" => Self::Required, | ||
| "TypeAlias" => Self::TypeAlias, | ||
| "TypeGuard" => Self::TypeGuard, | ||
| "TypeIs" => Self::TypeIs, | ||
| "ReadOnly" => Self::ReadOnly, | ||
| "Concatenate" => Self::Concatenate, | ||
| "NotRequired" => Self::NotRequired, | ||
| "LiteralString" => Self::LiteralString, | ||
| _ => return None, | ||
| }; | ||
|
|
||
| candidate | ||
| .check_module(file_to_module(db, file)?.known()?) | ||
| .then_some(candidate) | ||
| } | ||
|
|
||
| /// Return `true` if `module` is a module from which this `KnownInstance` variant can validly originate. | ||
| /// | ||
| /// Most variants can only exist in one module, which is the same as `self.class().canonical_module()`. | ||
| /// Some variants could validly be defined in either `typing` or `typing_extensions`, however. | ||
| pub fn check_module(self, module: KnownModule) -> bool { | ||
| match self { | ||
| Self::Any | ||
| | Self::ClassVar | ||
| | Self::Deque | ||
| | Self::List | ||
| | Self::Dict | ||
| | Self::DefaultDict | ||
| | Self::Set | ||
| | Self::FrozenSet | ||
| | Self::Counter | ||
| | Self::ChainMap | ||
| | Self::OrderedDict | ||
| | Self::Optional | ||
| | Self::Union | ||
| | Self::NoReturn | ||
| | Self::Tuple | ||
| | Self::Type | ||
| | Self::Callable => module.is_typing(), | ||
| Self::Annotated | ||
| | Self::Literal | ||
| | Self::LiteralString | ||
| | Self::Never | ||
| | Self::TypingSelf | ||
| | Self::Final | ||
| | Self::Concatenate | ||
| | Self::Unpack | ||
| | Self::Required | ||
| | Self::NotRequired | ||
| | Self::TypeAlias | ||
| | Self::TypeGuard | ||
| | Self::TypeIs | ||
| | Self::ReadOnly | ||
| | Self::TypeAliasType(_) | ||
| | Self::TypeVar(_) => { | ||
| matches!(module, KnownModule::Typing | KnownModule::TypingExtensions) | ||
| } |
There was a problem hiding this comment.
This is functionally the same, but in the try_from*() methods on the other Known* enums, we delay the file_to_module() call as much as possible, as it's probably much more expensive than matching on the value of a string. This PR does the same for this method.
|
carljm
approved these changes
Dec 19, 2024
|
1% speedup on the incremental benchmark... who knows if that's real, but certainly better than a slowdown whatever the case 😆 https://codspeed.io/astral-sh/ruff/branches/alex%2Fknown-module |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR does several things:
CoreStdlibModuleenum toKnownModule, since it is a parallel enum toKnownClass,KnownFunctionandKnownInstanceTypestdlib.rstomodule_resolver::module.rs. This allows it to be stored as a field onModuleInnerobjects, and retrieved using a getter method fromModuleobjects. This in turn means that we can then addknown()andis_known()methods to theModulestruct, which allows us to query whether a givenModulerepresents a known module in much the same way we already do usingFunctionType::is_knownandClass::is_known.These changes have several goals:
Known*enumsModulereally represents a specific module from the standard library. We had the same checks written out in several places, which was error-prone, since there are several things that needed to be checked and some are quite subtle. (One recent example: [red-knot] Statically known branches #15019 (comment).)definition.rsare an example)Test Plan
cargo test -p red_knot_python_semantic. No new tests added -- this should be a pure refactor.