Skip to content

Fix inference for same (but distinct) record types in seq CE - #16040

Merged
abonie merged 4 commits into
dotnet:mainfrom
abonie:fix-inf-seqce
Oct 9, 2023
Merged

Fix inference for same (but distinct) record types in seq CE#16040
abonie merged 4 commits into
dotnet:mainfrom
abonie:fix-inf-seqce

Conversation

@abonie

@abonieabonie commented Sep 25, 2023

Copy link
Copy Markdown
Member

Fixes#14213

The issue was, that with implicit yield, we first don't know whether an expression will have type unit or some particular expected type, so we type check it without any additional constraints, but that leads to ignoring type annotation and in case of #14213 we infer wrong type.

My solution so far is check the same expression again if we concluded it is not unit, this time with a subtype constraint.

@abonie
abonieforce-pushed the fix-inf-seqce branch 3 times, most recently from 435507d to 1762044CompareOctober 2, 2023 14:08
@abonie
abonieforce-pushed the fix-inf-seqce branch 2 times, most recently from 554cd5b to 2e257e1CompareOctober 4, 2023 14:52
@abonieabonie changed the title [WIP] Fix inference for same (but distinct) record types in seq CEFix inference for same (but distinct) record types in seq CEOct 5, 2023
Seems like it could be done better, right now there are two type
checking passes for same expression
This should be okay, because in both cases, the error message basically
says the same thing.
Comment threadsrc/Compiler/Checking/CheckComputationExpressions.fs
@abonie

Copy link
Copy Markdown
MemberAuthor

Weird that it failed CI after rebasing on main with errors related to deserialization

Fatal error. Internal CLR error. (0x80131506)
at System.Runtime.Serialization.SerializationGuard.<ThrowIfDeserializationInProgress>g__ThrowIfDeserializationInProgress|0_0(System.Runtime.Serialization.SerializationInfo, System.String, Int32 ByRef)

Will try to rerun

@abonie

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@abonie
abonie marked this pull request as ready for review October 5, 2023 15:37
@abonie
abonie requested a review from a team as a code ownerOctober 5, 2023 15:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Implicit yield and if-then when using records with similarly-named fields causes type inference error

4 participants

@abonie@psfinaki@edgarfgp@T-Gro