Skip to content

Commit f0c9378

Browse files
committed
Short-circuit validation of #if conditions after a versioned check.
When we encounter a check like `#if compiler(>=6.0) && something` or `#if swift(<6.0) || something`, and the left-hand term is a versioning check that determines the result of the whole condition, then we will not attempt to validate the right-hand term. This allows us to use versioned checks along with new discovery features (say, if we add an `#if attribute(x)`) without having to next conditions.
1 parent c755369 commit f0c9378

File tree

2 files changed

+62
-5
lines changed

2 files changed

+62
-5
lines changed

lib/Parse/ParseIfConfig.cpp

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,13 @@ static Expr *getSingleSubExp(ArgumentList *args, StringRef kindName,
133133
return nullptr;
134134
}
135135

136+
/// Returns \c true if the condition is a version check.
137+
static bool isVersionIfConfigCondition(Expr *Condition);
138+
139+
/// Evaluate the condition.
140+
/// \c true if success, \c false if failed.
141+
static bool evaluateIfConfigCondition(Expr *Condition, ASTContext &Context);
142+
136143
/// The condition validator.
137144
class ValidateIfConfigCondition :
138145
public ExprVisitor<ValidateIfConfigCondition, Expr*> {
@@ -202,7 +209,7 @@ class ValidateIfConfigCondition :
202209

203210
// We will definitely be consuming at least one operator.
204211
// Pull out the prospective RHS and slice off the first two elements.
205-
Expr *RHS = validate(S[1]);
212+
Expr *RHS = S[1];
206213
S = S.slice(2);
207214

208215
while (true) {
@@ -226,7 +233,7 @@ class ValidateIfConfigCondition :
226233

227234
OpName = NextOpName;
228235
Op = S[0];
229-
RHS = validate(S[1]);
236+
RHS = S[1];
230237
S = S.slice(2);
231238
}
232239

@@ -426,14 +433,37 @@ class ValidateIfConfigCondition :
426433
return E;
427434
}
428435

436+
Expr *visitBinaryExpr(BinaryExpr *E) {
437+
auto OpName = getDeclRefStr(E->getFn(), DeclRefKind::BinaryOperator);
438+
if (auto lhs = validate(E->getLHS())) {
439+
// If the left-hand side is a versioned condition, skip evaluation of
440+
// the right-hand side if it won't ever affect the result.
441+
if (OpName && isVersionIfConfigCondition(lhs)) {
442+
assert(*OpName == "&&" || *OpName == "||");
443+
bool isLHSTrue = evaluateIfConfigCondition(lhs, Ctx);
444+
if (isLHSTrue && *OpName == "||")
445+
return lhs;
446+
if (!isLHSTrue && *OpName == "&&")
447+
return lhs;
448+
}
449+
450+
E->getArgs()->setExpr(0, lhs);
451+
}
452+
453+
if (auto rhs = validate(E->getRHS()))
454+
E->getArgs()->setExpr(1, rhs);
455+
456+
return E;
457+
}
458+
429459
// Fold sequence expression for non-Swift3 mode.
430460
Expr *visitSequenceExpr(SequenceExpr *E) {
431461
ArrayRef<Expr*> Elts = E->getElements();
432-
Expr *foldedExpr = validate(Elts[0]);
462+
Expr *foldedExpr = Elts[0];
433463
Elts = Elts.slice(1);
434464
foldedExpr = foldSequence(foldedExpr, Elts);
435465
assert(Elts.empty());
436-
return foldedExpr;
466+
return validate(foldedExpr);
437467
}
438468

439469
// Other expression types are unsupported.
@@ -608,7 +638,6 @@ class IsVersionIfConfigCondition :
608638
bool visitExpr(Expr *E) { return false; }
609639
};
610640

611-
/// Returns \c true if the condition is a version check.
612641
static bool isVersionIfConfigCondition(Expr *Condition) {
613642
return IsVersionIfConfigCondition().visit(Condition);
614643
}

test/Parse/unknown_platform.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
// expected-error@+1{{unexpected platform condition}}
4+
#if hasGreeble(blah)
5+
#endif
6+
7+
// Future compiler, short-circuit right-hand side
8+
#if compiler(>=10.0) && hasGreeble(blah)
9+
#endif
10+
11+
// Current compiler, short-circuit right-hand side
12+
#if compiler(<10.0) || hasGreeble(blah)
13+
#endif
14+
15+
// This compiler, don't short-circuit.
16+
// expected-error@+1{{unexpected platform condition}}
17+
#if compiler(>=5.7) && hasGreeble(blah)
18+
#endif
19+
20+
// This compiler, don't short-circuit.
21+
// expected-error@+1{{unexpected platform condition}}
22+
#if compiler(<5.8) || hasGreeble(blah)
23+
#endif
24+
25+
// Not a "version" check, so don't short-circuit.
26+
// expected-error@+1{{unexpected platform condition}}
27+
#if os(macOS) && hasGreeble(blah)
28+
#endif

0 commit comments

Comments
 (0)