-
Notifications
You must be signed in to change notification settings - Fork 145
[Subtyping Generator] Switch to static-types for runtime subtype checking #4322
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.
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
base: master
Are you sure you want to change the base?
Conversation
interpreter/subtype_check.go
Outdated
| superTypeAccess := MustConvertStaticAuthorizationToSemaAccess(typeConverter, superTypeAuth) | ||
| subTypeAccess := MustConvertStaticAuthorizationToSemaAccess(typeConverter, subTypeAuth) | ||
| return sema.PermitsAccess(superTypeAccess, subTypeAccess) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conversion is very expensive. We should try to address this next.
Benchstat comparison
Results
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
493119f to
37527c7
Compare
This reverts commit d5cb8a2.
… into supun/subtype-gen-runtime
0107539 to
ac573c0
Compare
… into supun/subtype-gen-runtime
… into supun/subtype-gen-runtime
1a9e77a to
40dd843
Compare
40dd843 to
e3cc290
Compare
[Subtyping Generator] Merge master
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
I missed this before. Is this because in the interpreter we get sema types from the elaboration, and now have to convert them to static types? |
Closes #3691
Depends on #4335
Description
IsSubType()function (instead of the subtyping check based on sema-type)MustConvertStaticToSemaTypein VM, and rely on static-types.Results:
Vs VM in last released version (
v1.8.3)Vs VM before switching to static-types (master)
**Note: The master branch also contains some optimizations that was ported over. So the above comparison only includes the improvement we get from switching to static-types only.
Vs Interpreter (master)
Future work:
masterbranchFiles changedin the Github PR explorer