-
Notifications
You must be signed in to change notification settings - Fork 439
SwiftIfConfig improvements to suit usage in the compiler #2819
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
Changes from all commits
354e4fa
049ed84
5a19857
43ce0d6
98aeb44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
import SwiftSyntax | ||
|
||
/// Describes the ordering of a sequence of bytes that make up a word of | ||
/// storage for a particular architecture. | ||
|
@@ -114,15 +115,15 @@ public protocol BuildConfiguration { | |
/// information, which will translate into the `version` argument. | ||
/// | ||
/// - Parameters: | ||
/// - importPath: A nonempty sequence of identifiers describing the | ||
/// imported module, which was written in source as a dotted sequence, | ||
/// e.g., `UIKit.UIViewController` will be passed in as the import path | ||
/// array `["UIKit", "UIViewController"]`. | ||
/// - importPath: A nonempty sequence of (token, identifier) pairs | ||
/// describing the imported module, which was written in source as a | ||
/// dotted sequence, e.g., `UIKit.UIViewController` will be passed in as | ||
/// the import path array `[(token, "UIKit"), (token, "UIViewController")]`. | ||
/// - version: The version restriction on the imported module. For the | ||
/// normal `canImport(<import-path>)` syntax, this will always be | ||
/// `CanImportVersion.unversioned`. | ||
/// - Returns: Whether the module can be imported. | ||
func canImport(importPath: [String], version: CanImportVersion) throws -> Bool | ||
func canImport(importPath: [(TokenSyntax, String)], version: CanImportVersion) throws -> Bool | ||
Comment on lines
-125
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need both the token and the string? Couldn’t we extract the identifier from the token? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see. Makes sense. I think that explanation in a doc comment would be useful. |
||
|
||
/// Determine whether the given name is the active target OS (e.g., Linux, iOS). | ||
/// | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,90 @@ | |
import SwiftDiagnostics | ||
import SwiftSyntax | ||
|
||
/// Describes all of the #if/#elseif/#else clauses within the given syntax node, | ||
/// indicating their active state. This operation will recurse into all | ||
/// clauses to indicate regions of active / inactive / unparsed code. | ||
/// | ||
/// For example, given code like the following: | ||
/// #if DEBUG | ||
/// #if A | ||
/// func f() | ||
/// #elseif B | ||
/// func g() | ||
/// #elseif compiler(>= 12.0) | ||
/// please print the number after 41 | ||
/// #endif | ||
/// #else | ||
/// #endif | ||
/// | ||
/// If the configuration options `DEBUG` and `B` are provided, but `A` is not, | ||
/// and the compiler version is less than 12.0, the results will be contain: | ||
/// - Active region for the `#if DEBUG`. | ||
/// - Inactive region for the `#if A`. | ||
/// - Active region for the `#elseif B`. | ||
/// - Unparsed region for the `#elseif compiler(>= 12.0)`. | ||
/// - Inactive region for the final `#else`. | ||
public struct ConfiguredRegions { | ||
let regions: [Element] | ||
ahoppen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/// The set of diagnostics produced when evaluating the configured regions. | ||
public let diagnostics: [Diagnostic] | ||
|
||
/// Determine whether the given syntax node is active within the configured | ||
/// regions. | ||
public func isActive(_ node: some SyntaxProtocol) -> IfConfigRegionState { | ||
var currentState: IfConfigRegionState = .active | ||
for (ifClause, state) in regions { | ||
if node.position < ifClause.position { | ||
return currentState | ||
} | ||
|
||
if node.position >= ifClause.regionStart && node.position <= ifClause.endPosition { | ||
ahoppen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
currentState = state | ||
} | ||
} | ||
|
||
return currentState | ||
} | ||
} | ||
|
||
extension ConfiguredRegions: RandomAccessCollection { | ||
public typealias Element = (IfConfigClauseSyntax, IfConfigRegionState) | ||
public var startIndex: Int { regions.startIndex } | ||
public var endIndex: Int { regions.endIndex } | ||
|
||
public subscript(index: Int) -> Element { | ||
regions[index] | ||
} | ||
} | ||
|
||
extension ConfiguredRegions: CustomDebugStringConvertible { | ||
/// Provides source ranges for each of the configured regions. | ||
public var debugDescription: String { | ||
guard let firstRegion = first else { | ||
return "[]" | ||
} | ||
|
||
let root = firstRegion.0.root | ||
ahoppen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let converter = SourceLocationConverter(fileName: "", tree: root) | ||
let regionDescriptions = regions.map { (ifClause, state) in | ||
let startPosition = converter.location(for: ifClause.position) | ||
let endPosition = converter.location(for: ifClause.endPosition) | ||
return "[\(startPosition.line):\(startPosition.column) - \(endPosition.line):\(endPosition.column)] = \(state)" | ||
} | ||
|
||
return "[\(regionDescriptions.joined(separator: ", ")))]" | ||
} | ||
} | ||
|
||
extension IfConfigClauseSyntax { | ||
/// The effective start of the region after which code is subject to its | ||
/// condition. | ||
fileprivate var regionStart: AbsolutePosition { | ||
condition?.endPosition ?? elements?._syntaxNode.position ?? poundKeyword.endPosition | ||
} | ||
} | ||
|
||
extension SyntaxProtocol { | ||
/// Find all of the #if/#elseif/#else clauses within the given syntax node, | ||
/// indicating their active state. This operation will recurse into all | ||
|
@@ -39,10 +123,13 @@ extension SyntaxProtocol { | |
/// - Inactive region for the final `#else`. | ||
public func configuredRegions( | ||
in configuration: some BuildConfiguration | ||
) -> [(IfConfigClauseSyntax, IfConfigRegionState)] { | ||
) -> ConfiguredRegions { | ||
let visitor = ConfiguredRegionVisitor(configuration: configuration) | ||
visitor.walk(self) | ||
return visitor.regions | ||
return ConfiguredRegions( | ||
regions: visitor.regions, | ||
diagnostics: visitor.diagnostics | ||
) | ||
} | ||
} | ||
|
||
|
@@ -56,58 +143,111 @@ fileprivate class ConfiguredRegionVisitor<Configuration: BuildConfiguration>: Sy | |
/// Whether we are currently within an active region. | ||
var inActiveRegion = true | ||
|
||
/// Whether we are currently within an #if at all. | ||
var inAnyIfConfig = false | ||
|
||
// All diagnostics encountered along the way. | ||
var diagnostics: [Diagnostic] = [] | ||
|
||
init(configuration: Configuration) { | ||
self.configuration = configuration | ||
super.init(viewMode: .sourceAccurate) | ||
} | ||
|
||
override func visit(_ node: IfConfigDeclSyntax) -> SyntaxVisitorContinueKind { | ||
// If we're in an active region, find the active clause. Otherwise, | ||
// there isn't one. | ||
let activeClause = inActiveRegion ? node.activeClause(in: configuration).clause : nil | ||
// We are in an #if. | ||
let priorInAnyIfConfig = inAnyIfConfig | ||
inAnyIfConfig = true | ||
defer { | ||
inAnyIfConfig = priorInAnyIfConfig | ||
} | ||
|
||
// Walk through the clauses to find the active one. | ||
var foundActive = false | ||
var syntaxErrorsAllowed = false | ||
let outerState: IfConfigRegionState = inActiveRegion ? .active : .inactive | ||
for clause in node.clauses { | ||
// If we haven't found the active clause yet, syntax errors are allowed | ||
// depending on this clause. | ||
if !foundActive { | ||
syntaxErrorsAllowed = | ||
clause.condition.map { | ||
IfConfigClauseSyntax.syntaxErrorsAllowed($0).syntaxErrorsAllowed | ||
} ?? false | ||
} | ||
let isActive: Bool | ||
if let condition = clause.condition { | ||
if !foundActive { | ||
// Fold operators so we can evaluate this #if condition. | ||
let (foldedCondition, foldDiagnostics) = IfConfigClauseSyntax.foldOperators(condition) | ||
diagnostics.append(contentsOf: foldDiagnostics) | ||
Comment on lines
+174
to
+175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is now duplicating logic from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can do a little bit better here, because there were 4 copies of code like this. |
||
|
||
// In an active region, evaluate the condition to determine whether | ||
// this clause is active. Otherwise, this clause is inactive. | ||
// inactive. | ||
ahoppen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if inActiveRegion { | ||
let (thisIsActive, _, evalDiagnostics) = evaluateIfConfig( | ||
condition: foldedCondition, | ||
configuration: configuration | ||
) | ||
diagnostics.append(contentsOf: evalDiagnostics) | ||
|
||
// If this is the active clause, record it and then recurse into the | ||
// elements. | ||
if clause == activeClause { | ||
assert(inActiveRegion) | ||
// Determine if there was an error that prevented us from | ||
// evaluating the condition. If so, we'll allow syntax errors | ||
// from here on out. | ||
let hadError = | ||
foldDiagnostics.contains { diag in | ||
diag.diagMessage.severity == .error | ||
} | ||
|| evalDiagnostics.contains { diag in | ||
diag.diagMessage.severity == .error | ||
} | ||
|
||
regions.append((clause, .active)) | ||
if hadError { | ||
isActive = false | ||
syntaxErrorsAllowed = true | ||
} else { | ||
isActive = thisIsActive | ||
|
||
if let elements = clause.elements { | ||
walk(elements) | ||
// Determine whether syntax errors are allowed. | ||
syntaxErrorsAllowed = foldedCondition.allowsSyntaxErrorsFolded | ||
} | ||
} else { | ||
isActive = false | ||
|
||
// Determine whether syntax errors are allowed, even though we | ||
// skipped evaluation of the actual condition. | ||
syntaxErrorsAllowed = foldedCondition.allowsSyntaxErrorsFolded | ||
} | ||
} else { | ||
// We already found an active condition, so this is inactive. | ||
isActive = false | ||
} | ||
} else { | ||
// This is an #else. It's active if we haven't found an active clause | ||
// yet and are in an active region. | ||
isActive = !foundActive && inActiveRegion | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still need to determine #if DEBUG
print(debugDescription)
#elseif compiler(>99.0)
print debugDescription
#endif There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The compiler doesn't determine There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, that’s surprising to me but matching the compiler here for now makes sense for now to make switching between the two implementations easier. Should we gather these oddities that we find somewhere so we can consider fixing them in the future? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It turns out that this behavior changed in the compiler very recently (swiftlang/swift#74415). We could consider whether we need to fix the original issue in some other way. |
||
} | ||
|
||
foundActive = true | ||
continue | ||
// Determine and record the current state. | ||
let currentState: IfConfigRegionState | ||
switch (isActive, syntaxErrorsAllowed) { | ||
case (true, _): currentState = .active | ||
case (false, false): currentState = .inactive | ||
case (false, true): currentState = .unparsed | ||
} | ||
|
||
// If this is within an active region, or this is an unparsed region, | ||
// record it. | ||
if inActiveRegion || syntaxErrorsAllowed { | ||
regions.append((clause, syntaxErrorsAllowed ? .unparsed : .inactive)) | ||
// If there is a state change, record it. | ||
if !priorInAnyIfConfig || currentState != .inactive || currentState != outerState { | ||
regions.append((clause, currentState)) | ||
} | ||
|
||
// Recurse into inactive (but not unparsed) regions to find any | ||
// unparsed regions below. | ||
if !syntaxErrorsAllowed, let elements = clause.elements { | ||
// If this is a parsed region, recurse into it. | ||
if currentState != .unparsed, let elements = clause.elements { | ||
let priorInActiveRegion = inActiveRegion | ||
inActiveRegion = false | ||
inActiveRegion = isActive | ||
defer { | ||
inActiveRegion = priorInActiveRegion | ||
} | ||
walk(elements) | ||
} | ||
|
||
// Note when we found an active clause. | ||
if isActive { | ||
foundActive = true | ||
} | ||
} | ||
|
||
return .skipChildren | ||
|
Uh oh!
There was an error while loading. Please reload this page.