[SwiftLexicalLookup] Add support for #if and macro declarations. More ASTScope related fixes.
#2883
Conversation
…rt for macro declarations. Minor fixes.
|
@swift-ci Please test |
|
|
||
| /// Creates a new lookup configuration. | ||
| /// | ||
| /// - `finishInSequentialScope` - specifies whether lookup should finish | ||
| /// in the closest sequential scope. `false` by default. | ||
| @_spi(Experimental) public init( | ||
| finishInSequentialScope: Bool = false | ||
| finishInSequentialScope: Bool = false, | ||
| buildConfiguration: BuildConfiguration? = nil |
There was a problem hiding this comment.
It's probably worth taking in a ConfiguredRegions here rather than a BuildConfiguration. ConfiguredRegions caches results so we aren't re-evaluating the same #ifs repeatedly.
| if !containsExpressionSyntax && pattern.is(ExpressionPatternSyntax.self) { | ||
| containsExpressionSyntax = true | ||
| } | ||
|
|
There was a problem hiding this comment.
This is because we suppress the implicit error when there is some kind of pattern? Please leave such a comment with a code example to remind our future selves.
| } | ||
| } | ||
|
|
||
| @_spi(Experimental) extension IfConfigDeclSyntax: IntroducingToSequentialParentScopeSyntax, SequentialScopeSyntax { |
There was a problem hiding this comment.
This was quite straightforward, I love it!
|
@swift-ci please test Windows |
1 similar comment
|
@swift-ci please test Windows |
|
@swift-ci please test |
|
@swift-ci please test Windows |
| clause = | ||
| clauses | ||
| .first { clause in | ||
| clause.poundKeyword.tokenKind == .poundElse | ||
| } |
There was a problem hiding this comment.
Do we want to use the #else clause if no configured regions are specified? It’s a question. I don’t know what the correct answer here should be.
There was a problem hiding this comment.
Yes, that was my intention that when config.buildConfiguration is set to nil it should provide some "default" behavior. I think it makes sense to use #else as the only other logical solution here I can think of would be maybe not including any names from #if clauses? Either way, I'm open to debate and I'm curious what do you think about it.
There was a problem hiding this comment.
The only alternative I can think of would be to throw an error or something like that. @DougGregor Do you remember what we do in SwiftIfConfig when we’re unable to answer a query in the build configuration?
There was a problem hiding this comment.
SwiftIfConfig treats the condition as having failed if the configuration can't answer the query.
There was a problem hiding this comment.
OK, I guess that matches picking the #else clause then.
This PR includes more fixes that bring
SwiftLexicalLookupunqualified lookup implementation closer to theASTScopecounterpart. Those include:#ifclauses thanks toSwiftIfConfig..reversed()for iteration. Since it's more likely code block contains more variable decls than named decls, sequential lookup first goes through all items, and collects named decls.wildcardtoken filtering for names fromIdentifiableSyntax.With this PR, I was able to run validation for the entire Swift standard library with just two remaining bugs!