Sema: Optimize ConstraintSystem::is{Decl,Conformance}Unavailable #35132
Conversation
There was a problem hiding this comment.
This is almost exactly checkDeclarationAvailability besides availability reason computation at the end and isAwaysAvailable bit (does it make sense to add it there?). Should we just add a new method checkDeclarationAvailability which doesn't require ExprContext and returns a bool?
lib/Sema/ConstraintSystem.cpp
Outdated
| // of the versions on which the declaration is available. If this | ||
| // containment cannot be guaranteed, we say the reference is | ||
| // not available. | ||
| if (runningOSOverApprox.isContainedIn(safeRangeUnderApprox)) |
There was a problem hiding this comment.
Nit: return !runningOSOverApprox.isContainedIn(safeRangeUnderApprox);
|
@xedin It takes a ConstraintLocator too (notice how obtaining the source location is delayed as much as possible). Nothing else in TypeCheckAvailability.cpp deals with locators or other constraint system stuff. Are you sure it makes sense to put it there instead? |
Let's avoid creating an ExportContext, which computes a bunch of irrelevant stuff. Also, delay the expensive call to overApproximateAvailabilityAtLocation() unless we know the declaration is conditionally unavailable.
slavapestov
force-pushed
the
optimize-is-unavailable
branch
from
968b242 to
486562a
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
|
@slavapestov Getting SourceLoc from ASTNode is not an expensive operation, new method can just accept ASTNode as an argument. The main concern for me is that we'd have almost exactly the same logic in two different places. |
Let's avoid creating an ExportContext, which computes a bunch of
irrelevant stuff. Also, delay the expensive call to
overApproximateAvailabilityAtLocation() unless we know the
declaration is conditionally unavailable.