Skip to content

Circular validation cleanups, part 6 #27594

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

Merged

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Oct 10, 2019

The epic follow-up to #27571.

The following methods used to get the interface type and extract the result type out of it:

  • SubscriptDecl::getElementInterfaceType()
  • FuncDecl::getResultInterfaceType()
  • ConstructorDecl::getResultInterfaceType()

This PR refactors them into requests which directly compute the type, with the exception of the last one which is so trivial it is not a request in itself.

This PR also eliminates all computeType() methods, except for the one on AbstractFunctionDecl. We no longer eagerly compute the interface type of deserialized or imported declarations -- instead, its computed the first time we call setInterfaceType(), just like with a parsed declaration (but generally we do less work; for example, for an imported or deserialized function, the interface types of the parameters and result should already be set; getInterfaceType() just assembles the final (Generic)FunctionType).

@slavapestov slavapestov requested a review from CodaFi October 10, 2019 01:43
@slavapestov slavapestov force-pushed the circular-validation-cleanups-6 branch 2 times, most recently from 05175c4 to a7cc405 Compare October 10, 2019 06:39
In preparation for landing the ResultTypeRequest, let's compute
the result type directly from the declaration. For now, this still
forces getInterfaceType() to be computed, but that will go away
next.
…request

validateDecl() no longer explicitly validates the result TypeLoc.
Instead, the request is triggered via validateDecl() calling
computeType() on the FuncDecl or SubscriptDecl.
This is not the right long term solution -- we need to compute this
lazily using a request. But for now, let's stop computing it as a
side effect of calling validateDecl().
Even if we haven't computed an interface type yet, we might still be
able to compute one.
@slavapestov slavapestov force-pushed the circular-validation-cleanups-6 branch from a7cc405 to 90fa96d Compare October 10, 2019 23:55
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@@ -24,6 +24,7 @@
#include "swift/AST/PrettyStackTrace.h"
#include "swift/AST/SubstitutionMap.h"
#include "swift/Sema/IDETypeChecking.h"
#include "swift/Subsystems.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved createTypeChecker() from IDETypeChecking.h (which is still used in this file) to Subsystems.h.

Copy link
Contributor

@CodaFi CodaFi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility Debug

@theblixguy
Copy link
Collaborator

theblixguy commented Oct 12, 2019

Source compat debug failure is due to UPASS (noticed it on one of my PRs as well) - someone needs to un-XFAIL it

@slavapestov
Copy link
Contributor Author

apple/swift-lldb#2057
@swift-ci Please smoke test macOS

@slavapestov slavapestov merged commit 2e558f8 into swiftlang:master Oct 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants