Skip to content

Circular validation cleanups, part 3 #27546

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 4, 2019

This PR continues refactoring validateDecl() with the goal of replacing it with a request for computing a declaration's interface type.

One of the main differences between validateDecl() and requests is that on circularity, requests diagnose an error. On the other hand, validateDecl() would begin by checking if the declaration or any of its parent contexts were already being validated, and return silently without setting the declaration's type.

This PR refactors the circularity checking so that validateDecl() now only concerns itself with the declaration being validated, and not any of its parent contexts. A new method, ValueDecl::isRecursiveValidation() can be used to check if parent contexts are being validated. This is only meant to be a temporary fix, since all of its usages should be refactored to break cycles in a more principled manner.

For now, getInterfaceType() still returns an empty type if the declaration is being validated. The next PR will change this.

Another refactoring in this PR is that synthesizing opaque type declarations is now done using requests as well, instead of a side effect of calling validateDecl().

Instead of depending on a prior call to setOpaqueResultTypeDecl(),
we can instead just build the decl if we need one.
@slavapestov slavapestov force-pushed the circular-validation-cleanups-3 branch from 28a23af to a86acad Compare October 5, 2019 03:15
@slavapestov slavapestov requested a review from CodaFi October 5, 2019 03:15
@slavapestov slavapestov force-pushed the circular-validation-cleanups-3 branch from a86acad to 6edecd4 Compare October 5, 2019 03:16
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov slavapestov force-pushed the circular-validation-cleanups-3 branch from 6edecd4 to c3c70f3 Compare October 5, 2019 05:39
@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

This will be used as a transitional aid in refactoring getInterfaceType()
to always return a valid type, instead of silently returning Type() when
there is circularity.
Remove the early return in the case where one of our parent contexts was
being validated, and replace it with a simpler check that is only
performed in some callers related to associated type inference; we want
to bail out in one specific case only, which is that the declaration
is inside an extension whose generic signature is in the process of
being computed.
@slavapestov slavapestov force-pushed the circular-validation-cleanups-3 branch from c3c70f3 to 52cf365 Compare October 8, 2019 02:21
@slavapestov
Copy link
Contributor Author

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

@slavapestov
Copy link
Contributor Author

apple/swift-lldb#2035
@swift-ci Please smoke test Linux

@slavapestov slavapestov merged commit e7c0172 into swiftlang:master Oct 8, 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.

1 participant