Skip to content

Commit 354e4fa

Browse files
committed
Rework the configured regions computation to better match the compiler
The compiler's heuristics for inactive vs. unparsed regions differ from the implementation in SwiftIfConfig and have changed fairly recently. Rework the implementation here to match what the compiler does.
1 parent 16dc4f8 commit 354e4fa

File tree

2 files changed

+88
-31
lines changed

2 files changed

+88
-31
lines changed

Sources/SwiftIfConfig/ConfiguredRegions.swift

Lines changed: 67 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -56,58 +56,96 @@ fileprivate class ConfiguredRegionVisitor<Configuration: BuildConfiguration>: Sy
5656
/// Whether we are currently within an active region.
5757
var inActiveRegion = true
5858

59+
// All diagnostics encountered along the way.
60+
var diagnostics: [Diagnostic] = []
61+
5962
init(configuration: Configuration) {
6063
self.configuration = configuration
6164
super.init(viewMode: .sourceAccurate)
6265
}
6366

6467
override func visit(_ node: IfConfigDeclSyntax) -> SyntaxVisitorContinueKind {
65-
// If we're in an active region, find the active clause. Otherwise,
66-
// there isn't one.
67-
let activeClause = inActiveRegion ? node.activeClause(in: configuration).clause : nil
68+
// Walk through the clauses to find the active one.
6869
var foundActive = false
6970
var syntaxErrorsAllowed = false
7071
for clause in node.clauses {
71-
// If we haven't found the active clause yet, syntax errors are allowed
72-
// depending on this clause.
73-
if !foundActive {
74-
syntaxErrorsAllowed =
75-
clause.condition.map {
76-
IfConfigClauseSyntax.syntaxErrorsAllowed($0).syntaxErrorsAllowed
77-
} ?? false
78-
}
72+
let isActive: Bool
73+
if let condition = clause.condition {
74+
if !foundActive {
75+
// Fold operators so we can evaluate this #if condition.
76+
let (foldedCondition, foldDiagnostics) = IfConfigClauseSyntax.foldOperators(condition)
77+
diagnostics.append(contentsOf: foldDiagnostics)
7978

80-
// If this is the active clause, record it and then recurse into the
81-
// elements.
82-
if clause == activeClause {
83-
assert(inActiveRegion)
79+
// In an active region, evaluate the condition to determine whether
80+
// this clause is active. Otherwise, this clause is inactive.
81+
// inactive.
82+
if inActiveRegion {
83+
let (thisIsActive, _, evalDiagnostics) = evaluateIfConfig(
84+
condition: foldedCondition,
85+
configuration: configuration
86+
)
87+
diagnostics.append(contentsOf: evalDiagnostics)
8488

85-
regions.append((clause, .active))
89+
// Determine if there was an error that prevented us from
90+
// evaluating the condition. If so, we'll allow syntax errors
91+
// from here on out.
92+
let hadError =
93+
foldDiagnostics.contains { diag in
94+
diag.diagMessage.severity == .error
95+
}
96+
|| evalDiagnostics.contains { diag in
97+
diag.diagMessage.severity == .error
98+
}
8699

87-
if let elements = clause.elements {
88-
walk(elements)
89-
}
100+
if hadError {
101+
isActive = false
102+
syntaxErrorsAllowed = true
103+
} else {
104+
isActive = thisIsActive
90105

91-
foundActive = true
92-
continue
106+
// Determine whether syntax errors are allowed.
107+
syntaxErrorsAllowed = foldedCondition.allowsSyntaxErrorsFolded
108+
}
109+
} else {
110+
isActive = false
111+
112+
// Determine whether syntax errors are allowed, even though we
113+
// skipped evaluation of the actual condition.
114+
syntaxErrorsAllowed = foldedCondition.allowsSyntaxErrorsFolded
115+
}
116+
} else {
117+
// We already found an active condition, so this is inactive.
118+
isActive = false
119+
}
120+
} else {
121+
// This is an #else. It's active if we haven't found an active clause
122+
// yet and are in an active region.
123+
isActive = !foundActive && inActiveRegion
93124
}
94125

95-
// If this is within an active region, or this is an unparsed region,
96-
// record it.
97-
if inActiveRegion || syntaxErrorsAllowed {
98-
regions.append((clause, syntaxErrorsAllowed ? .unparsed : .inactive))
126+
// Determine and record the current state.
127+
let currentState: IfConfigRegionState
128+
switch (isActive, syntaxErrorsAllowed) {
129+
case (true, _): currentState = .active
130+
case (false, false): currentState = .inactive
131+
case (false, true): currentState = .unparsed
99132
}
133+
regions.append((clause, currentState))
100134

101-
// Recurse into inactive (but not unparsed) regions to find any
102-
// unparsed regions below.
103-
if !syntaxErrorsAllowed, let elements = clause.elements {
135+
// If this is a parsed region, recurse into it.
136+
if currentState != .unparsed, let elements = clause.elements {
104137
let priorInActiveRegion = inActiveRegion
105-
inActiveRegion = false
138+
inActiveRegion = isActive
106139
defer {
107140
inActiveRegion = priorInActiveRegion
108141
}
109142
walk(elements)
110143
}
144+
145+
// Note when we found an active clause.
146+
if isActive {
147+
foundActive = true
148+
}
111149
}
112150

113151
return .skipChildren

Tests/SwiftIfConfigTest/ActiveRegionTests.swift

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public class ActiveRegionTests: XCTestCase {
4949
"2️⃣": .active,
5050
"3️⃣": .inactive,
5151
"4️⃣": .unparsed,
52-
"5️⃣": .inactive,
52+
"5️⃣": .unparsed,
5353
"6️⃣": .active,
5454
]
5555
)
@@ -77,7 +77,7 @@ public class ActiveRegionTests: XCTestCase {
7777
"2️⃣": .active,
7878
"3️⃣": .inactive,
7979
"4️⃣": .unparsed,
80-
"5️⃣": .inactive,
80+
"5️⃣": .unparsed,
8181
"6️⃣": .active,
8282
]
8383
)
@@ -100,6 +100,25 @@ public class ActiveRegionTests: XCTestCase {
100100
]
101101
)
102102
}
103+
104+
func testActiveRegionUnparsed() throws {
105+
try assertActiveCode(
106+
"""
107+
#if false
108+
#if compiler(>=4.1)
109+
1️⃣let _: Int = 1
110+
#else
111+
// There should be no error here.
112+
2️⃣foo bar
113+
#endif
114+
#endif
115+
""",
116+
states: [
117+
"1️⃣": .unparsed,
118+
"2️⃣": .unparsed,
119+
]
120+
)
121+
}
103122
}
104123

105124
/// Assert that the various marked positions in the source code have the

0 commit comments

Comments
 (0)