Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve apply_type_tfunc accuracy #48329

Merged
merged 1 commit into from
Feb 1, 2023
Merged

Improve apply_type_tfunc accuracy #48329

merged 1 commit into from
Feb 1, 2023

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Jan 18, 2023 •
edited
Loading

closes #47089.

@aviatesk aviatesk added compiler:inference Type inference DO NOT MERGE Do not merge this PR! labels Jan 18, 2023
@aviatesk aviatesk mentioned this pull request Jan 18, 2023
Closed
@aviatesk aviatesk force-pushed the avi/improve-apply_type branch from b151a53 to 6defa30 Compare January 19, 2023 13:37
Keno added a commit that referenced this pull request Jan 26, 2023
@Keno
typelimits: Prevent accidental infinite growth in size limiting
99795fb 
There's a few related issues here:
1. Type size limiting depended on the identity of contained TypeVars.
   E.g. `Base.rewrap_unionall(Foo{Base.unwrap_unionall(A)}, A)`, would
   not be limited while the equivalent with typevars substituted would be.
   This is obviously undesirable because the identity of typevars is an
   implementation detail.
2. We were forcing `tupledepth` to 1 in the typevar case to allow replacing
   typevars by something concrete. However, this also allowed typevars being
   replaced by something derived from `sources`, which could be huge and itself
   contain TypeVars, allowing infinite growth.
3. There was a type query that was run on types containg free typevars. This
   would have asserted in a debug build and gave wrong answers in a release build.

Fix all three issues, which together addresses an inference non-termination seen in #48329.
@Keno Keno mentioned this pull request Jan 26, 2023
Merged
Keno added a commit that referenced this pull request Jan 28, 2023
@Keno
typelimits: Prevent accidental infinite growth in size limiting
6564216 
There's a few related issues here:
1. Type size limiting depended on the identity of contained TypeVars.
   E.g. `Base.rewrap_unionall(Foo{Base.unwrap_unionall(A)}, A)`, would
   not be limited while the equivalent with typevars substituted would be.
   This is obviously undesirable because the identity of typevars is an
   implementation detail.
2. We were forcing `tupledepth` to 1 in the typevar case to allow replacing
   typevars by something concrete. However, this also allowed typevars being
   replaced by something derived from `sources`, which could be huge and itself
   contain TypeVars, allowing infinite growth.
3. There was a type query that was run on types containg free typevars. This
   would have asserted in a debug build and gave wrong answers in a release build.

Fix all three issues, which together addresses an inference non-termination seen in #48329.
@aviatesk aviatesk force-pushed the avi/improve-apply_type branch from 6defa30 to cfcafb9 Compare January 30, 2023 05:22
@aviatesk aviatesk changed the base branch from master to kf/badsizelimit January 30, 2023 05:23
@aviatesk aviatesk force-pushed the avi/improve-apply_type branch from cfcafb9 to 7d8d6bc Compare January 30, 2023 06:20
@Keno
Copy link
Member

Keno commented Jan 30, 2023

@nanosoldier runbenchmarks("inference", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - no performance regressions were detected. A full report can be found here.

@Keno Keno changed the title wip: fix apply_type_tfunc accuracy Improve apply_type_tfunc accuracy Feb 1, 2023
@Keno Keno removed the DO NOT MERGE Do not merge this PR! label Feb 1, 2023
@Keno Keno merged commit 4e8a388 into kf/badsizelimit Feb 1, 2023
@Keno Keno deleted the avi/improve-apply_type branch February 1, 2023 14:17
DilumAluthge pushed a commit that referenced this pull request Feb 4, 2023
@Keno @DilumAluthge
typelimits: Prevent accidental infinite growth in size limiting
8de936f 
There's a few related issues here:
1. Type size limiting depended on the identity of contained TypeVars.
   E.g. `Base.rewrap_unionall(Foo{Base.unwrap_unionall(A)}, A)`, would
   not be limited while the equivalent with typevars substituted would be.
   This is obviously undesirable because the identity of typevars is an
   implementation detail.
2. We were forcing `tupledepth` to 1 in the typevar case to allow replacing
   typevars by something concrete. However, this also allowed typevars being
   replaced by something derived from `sources`, which could be huge and itself
   contain TypeVars, allowing infinite growth.
3. There was a type query that was run on types containg free typevars. This
   would have asserted in a debug build and gave wrong answers in a release build.

Fix all three issues, which together addresses an inference non-termination seen in #48329.
DilumAluthge pushed a commit that referenced this pull request Feb 4, 2023
@aviatesk @DilumAluthge
wip: fix apply_type_tfunc accuracy (#48329)
b77fab2
DilumAluthge pushed a commit that referenced this pull request Feb 5, 2023
@Keno @aviatesk
typelimits: Prevent accidental infinite growth in size limiting (#48421)
51e3bc3 
* typelimits: Prevent accidental infinite growth in size limiting

There's a few related issues here:
1. Type size limiting depended on the identity of contained TypeVars.
   E.g. `Base.rewrap_unionall(Foo{Base.unwrap_unionall(A)}, A)`, would
   not be limited while the equivalent with typevars substituted would be.
   This is obviously undesirable because the identity of typevars is an
   implementation detail.
2. We were forcing `tupledepth` to 1 in the typevar case to allow replacing
   typevars by something concrete. However, this also allowed typevars being
   replaced by something derived from `sources`, which could be huge and itself
   contain TypeVars, allowing infinite growth.
3. There was a type query that was run on types containg free typevars. This
   would have asserted in a debug build and gave wrong answers in a release build.

Fix all three issues, which together addresses an inference non-termination seen in #48329.

* wip: fix `apply_type_tfunc` accuracy (#48329)

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
@aviatesk aviatesk mentioned this pull request Mar 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
compiler:inference Type inference
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

improve inference of keys(::Dict{String}) to fix invalidations
3 participants
@aviatesk @Keno @nanosoldier