Drastically Simplify VarDecl Validation #27648
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
apple/swift-lldb#2058 |
|
@swift-ci please test source compatibility |
CodaFi
commented
Oct 12, 2019
| @@ -4915,6 +4908,9 @@ class VarDecl : public AbstractStorageDecl { | |||
| Parent = v; | |||
| } | |||
|
|
|||
| NamedPattern *getNamingPattern() const { return NamingPattern; } | |||
There was a problem hiding this comment.
The naming pattern is a compromise and a cache. We could absolutely requestify this and have it computed by rooting around in the parent pattern (binding) for the NamedPattern that has us/our canonical var as its decl.
|
Build failed |
|
Build failed |
|
apple/swift-lldb#2058 |
CodaFi
force-pushed
the
unbounded-progress
branch
from
61d7236 to
e27f932
Compare
|
apple/swift-lldb#2058 |
|
@swift-ci please test source compatibility |
|
Build failed |
VarDecls inherit their TypeRepr from their enclosing pattern, if any. To retrieve the TypeRepr attached to a VarDecl or a ParamDecl in a uniform way, the getTypeReprOrParentPatternTypeRepr API has been provided.
This is an amalgam of simplifications to the way VarDecls are checked and assigned interface types. First, remove TypeCheckPattern's ability to assign the interface and contextual types for a given var decl. Instead, replace it with the notion of a "naming pattern". This is the pattern that semantically binds a given VarDecl into scope, and whose type will be used to compute the interface type. Note that not all VarDecls have a naming pattern because they may not be canonical. Second, remove VarDecl's separate contextual type member, and force the contextual type to be computed the way it always was: by mapping the interface type into the parent decl context. Third, introduce a catch-all diagnostic to properly handle the change in the way that circularity checking occurs. This is also motivated by TypeCheckPattern not being principled about which parts of the AST it chooses to invalidate, especially the parent pattern and naming patterns for a given VarDecl. Once VarDecls are invalidated along with their parent patterns, a large amount of this diagnostic churn can disappear. Unfortunately, if this isn't here, we will fail to catch a number of obviously circular cases and fail to emit a diagnostic.
Overload resolution performs a lookup rooted at a pattern binding's initializer that scoops up the var decl bound by the pattern. This forces it to validate the variable while type checking said variable's initializer. The old answer to this problem was to skip validation which returns a temporary ErrorType. We should patch lookup so it doesn't consider these variables.
|
The Sourcery failure doesn't appear to be mine. Gonna rebase and try again. |
CodaFi
force-pushed
the
unbounded-progress
branch
from
e27f932 to
e4bf3a6
Compare
Inline the interface type reset into its callers and make sure they're also setting the invalid bit - which this was not doing before. Unfortunately, this is not enough to be able to simplify any part of var decl validation.
CodaFi
force-pushed
the
unbounded-progress
branch
from
e4bf3a6 to
0082ba5
Compare
|
@swift-ci please smoke test |
|
apple/swift-lldb#2058 |
|
@swift-ci please test source compatibility Debug |
|
UPASS! ⛵️ |
CodaFi
added a commit
to CodaFi/swift
that referenced
this pull request
Oct 15, 2019
Because of the cleanup in swiftlang#27648, it is now possible to just grab the right type repr here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an amalgam of simplifications to the way VarDecls are checked
and assigned interface types.
First, remove TypeCheckPattern's ability to assign the interface and
contextual types for a given var decl. Instead, replace it with the
notion of a "naming pattern". This is the pattern that semantically
binds a given VarDecl into scope, and whose type will be used to compute
the interface type. Note that not all VarDecls have a naming pattern
because they may not be canonical.
Second, remove VarDecl's separate contextual type member, and force the
contextual type to be computed the way it always was: by mapping the
interface type into the parent decl context.
Third, introduce a catch-all diagnostic to properly handle the change in
the way that circularity checking occurs. This is also motivated by
TypeCheckPattern not being principled about which parts of the AST it
chooses to invalidate, especially the parent pattern and naming patterns
for a given VarDecl. Once VarDecls are invalidated along with their
parent patterns, a large amount of this diagnostic churn can disappear.
Unfortunately, if this isn't here, we will fail to catch a number of
obviously circular cases and fail to emit a diagnostic.