☣️[Sema] Transform initializers into coercions for conforming numeric literals #15311
Conversation
xedin
force-pushed
the
transform-constructors-into-coercions
branch
from
4a1da85
to
7d9c2b8
Compare
|
@swift-ci please test source compatibility |
|
🤞 |
|
This is a potentially source-breaking change, although it's likely users would have wanted the new behavior all along. Previously, the possibility of warning the user in these cases was discussed. However, surely, it can't be sound to have silently custom type inference rules specifically for these initializers and not other uses of literals as arguments. Have you considered formally adopting the rule you're using above for all literal arguments (and, ideally, extending them to operators too)? That'd fix the heterogeneous-comparison-over-homogeneous-comparison-in-generic-code issue too! |
…iterals To speed up type-checking for numeric literal types let's try to implicitly convert expressions like `UInt32(42)` into `42 as UInt32` but only if requested type conforms to the correct literal protocol. The desired outcome is that we can properly type-check expressions like `UInt64(0xffff_ffff_ffff_ffff)` and speed up solving of more complex expressions by avoiding checking all of the initializer overloads since most of them are likely unrelated.
xedin
force-pushed
the
transform-constructors-into-coercions
branch
from
7d9c2b8
to
4039e0f
Compare
|
@xwu The idea is to verify that this is going to help type-checker and other things, and if so then go through formal evolution process. |
xedin
changed the title
|
@swift-ci please test source compatibility |
|
Only one project failed with an error I haven't see before: |
|
@swift-ci please smoke benchmark compiler performance |
|
@graydon @shahmishal Any idea why compiler performance CI isn't triggering? |
|
Ok, it's just me being stupid... |
|
@swift-ci Please smoke test compiler performance |
|
@swift-ci please test compiler performance |
1 similar comment
|
@swift-ci please test compiler performance |
Build comment file:Summary for master fullUnexpected test results, excluded stats for ChattoAdditions, CoreStore, SourceKittenFramework, StencilSwiftKit, AMScrollingNavbar, ProcedureKit, GRDB, JSQDataSourcesKit, AsyncNinja, ReactiveCocoa, IBAnimatable, ObjectMapper No regressions above thresholds Debugdebug briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
debug detailedRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
Debug-optdebug-opt briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
debug-opt detailedRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
Wmo-ononewmo-onone briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
wmo-onone detailedRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
Releaserelease briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
release detailedRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
|
|
I'll try another way where instead of converting to coercion I'm just going to favor overloads which satisfy protocol requirement related to literals, that should work for every literal protocol and be less intrusive (but would still require swift-evolution proposal). |
|
@xwu I've created a pitch for these changes - https://forums.swift.org/t/pitch-literal-initialization-via-coercion/11251 (going to open separate PR if everything goes fine). |
|
Instead of rewriting this in the precheck pass, have you considered just generating different constraints in CSApply? That would keep the AST aligned with what the user wrote. I realize that that would make it a trickier patch, I'm just wondering if you considered the tradeoffs and discussed it with @rudkx ? |
|
@lattner Yes, we've discussed it with @rudkx and @DougGregor and came to the conclusion that |
|
@lattner I'm curious if you have specific benefits in mind to implementing this completely in the constraint system rather than doing this as a sort of desugaring during |
|
I don't have specific goals, other than keeping the ASTs more closely aligned to source code. This tends to matter for refactoring and source transformation'y things, and is an (academically) more pure way to go. |
|
For what it's worth I can look into how to do the same thing when we generate overload sets for constructors, so the overload for the literal protocol requirement is favored in this situation, which should produce exactly the same results. |
|
@lattner I assumed you might be thinking of one or more of those concerns but didn't want to be a mind-reader so I thought I would ask. @xedin I wonder if we could just bind the appropriate overload and effectively treat |
| return nullptr; | ||
|
|
||
| auto *argExpr = call->getArg()->getSemanticsProvidingExpr(); | ||
| auto *number = dyn_cast<NumberLiteralExpr>(argExpr); |
There was a problem hiding this comment.
Shouldn't this apply to all literal kinds? For example, Set([1, 2, 3])?
There was a problem hiding this comment.
Yes, I just tried with NumberLiteralExpr because I ran into problem with failable initializer in swiftpm clashing with ExpressibleByStringLiteral but still wanted to get some performance numbers, and NumberLiteralExpr since to be the biggest pain point for stdlib folks anyway.
| auto result = TC.conformsToProtocol(type, protocol, DC, options); | ||
| if (result) { | ||
| auto *expr = | ||
| new (TC.Context) CoerceExpr(argExpr, {}, typeExpr->getTypeRepr()); |
There was a problem hiding this comment.
We're losing a bunch of the source information in doing this. Should we try to make this a constraint-system change and have CSApply fix up the result, rather than turning it into a CoerceExpr?
There was a problem hiding this comment.
Yes, that's on my agenda to do next.
To speed up type-checking for numeric literal types let's try to implicitly
convert expressions like
UInt32(42)into42 as UInt32but onlyif requested type conforms to the correct literal protocol.
The desired outcome is that we can properly type-check expressions
like
UInt64(0xffff_ffff_ffff_ffff)and speed up solving of morecomplex expressions by avoiding checking all of the initializer
overloads since most of them are likely unrelated.