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

[SR-10331] Dedupe @dynamicCallable implementation for "visiting all supertypes" #52731

Open
dan-zheng opened this issue Apr 7, 2019 · 2 comments
Open

[SR-10331] Dedupe @dynamicCallable implementation for "visiting all supertypes" #52731

dan-zheng opened this issue Apr 7, 2019 · 2 comments
Labels
compiler The Swift compiler in itself task

Comments

@dan-zheng
Copy link
Collaborator

Previous ID SR-10331
Radar None
Original Reporter @dan-zheng
Type Task
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Task
Assignee None
Priority Medium

md5: 8df931a0959a99829fa7c155878a472e

Issue Description:

CSSimplify.cpp contains custom logic for "visiting all supertypes" for dynamic callable constraint simplification: getDynamicCallableMethods is used to visit all "supertypes" (superclasses and inherited protocols) of a @dynamicCallable type to find func dynamicallyCall special syntax delegate methods.

This duplicates not only lookupQualified (which should be the canonical implementation of such logic), but also bits of code in TypeCheckAttr.cpp (visitDynamicCallableAttr).

We should remove custom supertype traversal logic from getDynamicCallableMethods and re-implement using lookupQualified and related functions.
@dan-zheng
Copy link
Collaborator Author

I've been thinking about this in the back of my head for months.

Tentative solution: unify logic between TypeCheckAttr.cpp and CSSimplify.cpp using TC.lookupMember.

  • visitDynamicCallableAttr calls TC.lookupMember to find func dynamicallyCall methods. I believe TC.lookupMember in turn calls lookupQualified, which performs supertype traversal - so the custom traversal logic in CSSimplify.cpp is unnecessary.

  • Unifying logic between CSSimplify.cpp and TypeCheckAttr.cpp just makes sense - code in both files can cache a type's func dynamicallyCall methods for efficiency.

  • Currently, caching logic exists only for CSSimplify, using a cache defined on ConstraintSystem. Code in TypeCheckAttr.cpp only has access to a TypeChecker, not a ConstraintSystem, so the cache should perhaps be moved to TypeChecker (or ASTContext).

@dan-zheng
Copy link
Collaborator Author

cc @slavapestov, who has been unhappy with this for months - me too
cc pschuh (JIRA User), who indicated interest in taking this on

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler The Swift compiler in itself task
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@dan-zheng