[SwiftLexicalLookup][GSoC] Fix Swift 5.8 compatibility and update documentation. #2755
Conversation
There was a problem hiding this comment.
Thanks for addressing the comments in a separate PR @MAJKFL! That makes it a lot easier to review and I hope that it’s not too much trouble if you have merge conflicts with #2719.
- Would you like to also include the test case from [SwiftLexicalLookup][GSoC] Add initial name lookup functionality #2719 (comment) in this PR?
- Could you also address [SwiftLexicalLookup][GSoC] Add initial name lookup functionality #2719 (comment)
Out-of-scope for this follow-up PR but still open from #2719 (mostly so I can find it again)
- Handling of
#if[SwiftLexicalLookup][GSoC] Add initial name lookup functionality #2719 (comment) - Lookup using
StringvsIdentifier[SwiftLexicalLookup][GSoC] Add initial name lookup functionality #2719 (comment)
Sources/SwiftLexicalLookup/Configurations/FileScopeNameIntroductionStrategy.swift
Outdated
Show resolved
Hide resolved
| func does(name: String?, referTo introducedName: LookupName, at syntax: SyntaxProtocol) -> Bool { | ||
| introducedName.isAccessible(at: syntax) && (name == nil || introducedName.refersTo(name!)) | ||
| } |
There was a problem hiding this comment.
I can’t say that I’m a fan of having does as the base name of a function. Was the a specific reason why you extracted this?
There was a problem hiding this comment.
What would you say about checkName( , refersTo: ,at: )? It doesn't really do much in this PR, but once we start modeling more complex scopes (like in #2748) this method will be used all over the place so it's convenient to have it extracted.
There was a problem hiding this comment.
Oh, I see. checkName sounds good to me.
I already added a test case for
Thank you for catching the missing doc. I missed it when copying the changes from #2748 |
|
@swift-ci Please test |
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
|
@MAJKFL any reason not to merge this now? |
|
I don't have anything more to add. I think it should be good to go. |
|
Okay, merged! |
…okup." This reverts commit dda31f9.
This PR addresses requested changes by @ahoppen from #2719 that were not included before its merging. Those include mainly fixing Swift 5.8 compatibility and documentation enhancements.