Skip to content

Conversation

@A5rocks
Copy link
Collaborator

@A5rocks A5rocks commented Jan 17, 2026

Followup to #20492. This is basically just refactoring based on what seems more correct.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Does this have any behavior change? If yes, consider adding a test case.

Key: _TypeAlias = tuple[Any, ...]
LITERAL_NAMES = frozenset(
("builtins.True", "builtins.False", "builtins.None", "builtins.NotImplemented")
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

As micro-optimization, can you make this Final? This is potentially perfomance-critical, and Final will make lookups faster when using mypyc.

if e.fullname in LITERAL_NAMES:
return LITERAL_YES
elif isinstance(e.node, TypeAlias) and not e.node.python_3_12_type_alias:
if is_named_instance(e.node.target, LITERAL_NAMES):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can a type alias refer to anything but None out of the options here? If this is case, maybe avoid LITERAL_NAMES.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants