[ty] Only perform literal promotion for implicitly inferred literals#23107
Merged
ibraheemdev merged 4 commits intomainfrom Feb 19, 2026
Merged
[ty] Only perform literal promotion for implicitly inferred literals#23107ibraheemdev merged 4 commits intomainfrom
ibraheemdev merged 4 commits intomainfrom
Conversation
ibraheemdev
requested review from
AlexWaygood,
BurntSushi,
Gankra,
MichaReiser,
carljm,
dcreager and
sharkdp
as code owners
ibraheemdev
commented
Feb 5, 2026
|
|
||
| // For a given type variable, we keep track of the variance of any assignments to | ||
| // that type variable in the type context. | ||
| let mut variance_in_arguments: FxHashMap<BoundTypeVarIdentity<'_>, TypeVarVariance> = |
Member
Author
There was a problem hiding this comment.
In theory, we still have to avoid promoting literals assigned to a type variable that are is invariant position in any parameter type to avoid assignability errors. However, because we perform literal promotion for all invariant type variables in a generic constructor, the only way you can get a literal type in invariant position is through an explicit literal annotation, in which case we will not attempt promotion anyways.
As per @dcreager, the new constraint solver should give us the necessary information to determine whether or not literal promotion is possible (via the lower and upper bounds of the type assignment), so any edge cases in this area (if there are any) should be fixed then anyways.
ibraheemdev
force-pushed
the
ibraheem/literal-origins
branch
from
5919567 to
fb64628
Compare
Typing conformance resultsNo changes detected ✅ |
ibraheemdev
commented
Feb 5, 2026
| diagnostic_builder.into_diagnostic(format_args!("Unsupported `{}` operation", error.op)); | ||
|
|
||
| if left_ty == right_ty { | ||
| if left_ty.is_equivalent_to(db, right_ty) { |
Member
Author
There was a problem hiding this comment.
This doesn't seem exactly correct, this diagnostic might really want type equality, not equivalence. However, the promotable and unpromotable form of the same literal value are not equal by salsa ID equality, so this can lead to odd diagnostics (e.g., "values are of types Literal[1] and Literal[1]"). Maybe we need another semantic equality operation for cases like this?
Member
There was a problem hiding this comment.
From what I can tell, this change seems to lead to a lot of diagnostics improving their error message on e.g. pandas TBH (where there are two operands with differently ordered, but equivalent, unions, the error message is much improved as a result of this change IMO). It could even be worth pulling it out as a standalone change to reduce the ecosystem diff on this PR
|
ibraheemdev
commented
Feb 5, 2026
| (Type::LiteralValue(literal), Type::NominalInstance(instance)) | ||
| if literal.is_string(db) => | ||
| { | ||
| let value = literal.as_string(db).unwrap(); |
Member
Author
There was a problem hiding this comment.
Unfortunately, there are a lot of places where we have to unwrap like this due to the lack of if let guards.
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
0 | 24 | 30 |
invalid-await |
0 | 40 | 0 |
unsupported-operator |
0 | 0 | 23 |
unresolved-attribute |
0 | 0 | 10 |
no-matching-overload |
0 | 6 | 0 |
invalid-assignment |
0 | 4 | 1 |
invalid-return-type |
0 | 4 | 0 |
unused-type-ignore-comment |
2 | 1 | 0 |
type-assertion-failure |
0 | 1 | 0 |
| Total | 2 | 80 | 64 |
ibraheemdev
force-pushed
the
ibraheem/literal-origins
branch
4 times, most recently
from
b33fd11 to
f749fe8
Compare
Member
Author
|
Hmm.. a lot of the ecosystem changes seem to have gone away after rebasing on #23100. |
AlexWaygood
reviewed
Feb 6, 2026
Member
Author
|
I wonder if it makes sense to switch the layout so that unpromotable literals do not allocate, which is the type we infer for any annotated literal types (e.g., function parameters or return values). I assumed those would be rarer, but I'm not exactly sure where the performance regression is coming from. |
Member
|
You could also possibly experiment with always interning the data held by int-Literals, keeping bool Literals always uninterned, and not distinguishing between promotable vs unpromotable Literals when it comes to whether they are interned |
ibraheemdev
force-pushed
the
ibraheem/literal-origins
branch
from
d67da9c to
1e94b0f
Compare
Member
Author
|
I removed the interning for both literal types, but the walltime benchmarks seem unaffected. The performance regression seems to be coming from elsewhere, maybe the type mapping? |
Member
Author
|
That seems to have done it. |
AlexWaygood
reviewed
Feb 6, 2026
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
Awesome work getting rid of the perf regression!
| | Type::StringLiteral(_) | ||
| | Type::BytesLiteral(_) | ||
| | Type::EnumLiteral(_) => match type_mapping { | ||
| Type::ModuleLiteral(_) => match type_mapping { |
Member
There was a problem hiding this comment.
I'm not sure we should ever be promoting module-literal types TBH, but I guess that's a separate change...
ibraheemdev
force-pushed
the
ibraheem/literal-origins
branch
2 times, most recently
from
ccc8647 to
506cbf5
Compare
Memory usage reportSummary
Significant changesClick to expand detailed breakdownflake8
trio
sphinx
prefect
|
carljm
approved these changes
Feb 13, 2026
Contributor
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
Nice work! LGTM. I haven't looked at the ecosystem; trusting that you have and it looks OK.
crates/ty_python_semantic/resources/mdtest/literal_promotion.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/literal_promotion.md
Outdated
Show resolved
Hide resolved
ibraheemdev
force-pushed
the
ibraheem/literal-origins
branch
from
506cbf5 to
f414505
Compare
Member
Author
|
There seem to be a couple new false positives on rotki, looking into those now. |
Member
Author
|
Ecosystem results look much better now. Note that we are no longer using the |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Updates our literal promotion heuristics to only promote implicitly inferred literal values, i.e., literal values inferred without an explicit literal annotation. This requires us to track promotability on the literal value type itself, and so a lot of the changes here are churn from having to split the literal value variants of
Typeinto a separateLiteralValuetype.Another consequence of this change is that the same literal value does not compare equal in its promotable and unpromotable form, so there are a few cases where we relied on type equality (or hashing) that need to be changed.
Resolves astral-sh/ty#2664, astral-sh/ty#1815, and supersedes #22322.