-
Notifications
You must be signed in to change notification settings - Fork 440
[SwiftLexicalLookup] Composite names #2905
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
Conversation
@swift-ci Please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
case .x(let 1️⃣a, let 2️⃣b), .y(let 3️⃣a, let 4️⃣b): | ||
print(5️⃣x) | ||
case .z(let 7️⃣a), .smth(let 8️⃣a): | ||
print(9️⃣x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn’t this be print(a)
? x
is never defined in this test case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I purposefully called it x
to further indicate that we are not performing name matching in this case. We use useNilAsTheParameter: true
for that. This test is supposed to check if we properly partition names equivalent names (like in the first case here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I missed that and thought that we were looking up x
, not that we were collecting all names available at that location. The test makes sense now 😄
…Description` string concatenation for `multipleNames` case.
@swift-ci Please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍🏽
@swift-ci Please test Windows |
1 similar comment
@swift-ci Please test Windows |
@swift-ci Please test |
@swift-ci Please test Windows |
@swift-ci please test platform Windows |
1 similar comment
@swift-ci please test platform Windows |
@swift-ci please test Windows |
This PR introduces notion of composite names to
SwiftLexicalLookup
. In Swift, it’s possible to have a switch case introducing one name for several enum cases:ASTScope
currently deals with this special case by just introducing the first name from the list. Composite names are supposed to instead merge all names with the same semantic meaning into one, unified name representation. It stores an array of related names and, to match the behavior of the compiler, a composite name is represented by the first name.Combined with some additional minor fixes included in this PR, I was able to finally perform entire Swift compilation with fully enabled validation! There still seem to be some discrepancies when running entire test suite with
lit
, but I’m happy to see the library finally maturing.