Skip to content

Commit

Permalink
Short-circuit validation of #if conditions after a versioned check.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
DougGregor committed Jul 20, 2022
1 parent 569859a commit 2da2561
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 5 deletions.
39 changes: 34 additions & 5 deletions lib/Parse/ParseIfConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ static Expr *getSingleSubExp(ArgumentList *args, StringRef kindName,
return nullptr;
}

/// Returns \c true if the condition is a version check.
static bool isVersionIfConfigCondition(Expr *Condition);

/// Evaluate the condition.
/// \c true if success, \c false if failed.
static bool evaluateIfConfigCondition(Expr *Condition, ASTContext &Context);

/// The condition validator.
class ValidateIfConfigCondition :
public ExprVisitor<ValidateIfConfigCondition, Expr*> {
Expand Down Expand Up @@ -202,7 +209,7 @@ class ValidateIfConfigCondition :

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

while (true) {
Expand All @@ -226,7 +233,7 @@ class ValidateIfConfigCondition :

OpName = NextOpName;
Op = S[0];
RHS = validate(S[1]);
RHS = S[1];
S = S.slice(2);
}

Expand Down Expand Up @@ -426,14 +433,37 @@ class ValidateIfConfigCondition :
return E;
}

Expr *visitBinaryExpr(BinaryExpr *E) {
auto OpName = getDeclRefStr(E->getFn(), DeclRefKind::BinaryOperator);
if (auto lhs = validate(E->getLHS())) {
// If the left-hand side is a versioned condition, skip evaluation of
// the right-hand side if it won't ever affect the result.
if (OpName && isVersionIfConfigCondition(lhs)) {
assert(*OpName == "&&" || *OpName == "||");
bool isLHSTrue = evaluateIfConfigCondition(lhs, Ctx);
if (isLHSTrue && *OpName == "||")
return lhs;
if (!isLHSTrue && *OpName == "&&")
return lhs;
}

E->getArgs()->setExpr(0, lhs);
}

if (auto rhs = validate(E->getRHS()))
E->getArgs()->setExpr(1, rhs);

return E;
}

// Fold sequence expression for non-Swift3 mode.
Expr *visitSequenceExpr(SequenceExpr *E) {
ArrayRef<Expr*> Elts = E->getElements();
Expr *foldedExpr = validate(Elts[0]);
Expr *foldedExpr = Elts[0];
Elts = Elts.slice(1);
foldedExpr = foldSequence(foldedExpr, Elts);
assert(Elts.empty());
return foldedExpr;
return validate(foldedExpr);
}

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

/// Returns \c true if the condition is a version check.
static bool isVersionIfConfigCondition(Expr *Condition) {
return IsVersionIfConfigCondition().visit(Condition);
}
Expand Down
28 changes: 28 additions & 0 deletions test/Parse/unknown_platform.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// RUN: %target-typecheck-verify-swift

// expected-error@+1{{unexpected platform condition}}
#if hasGreeble(blah)
#endif

// Future compiler, short-circuit right-hand side
#if compiler(>=10.0) && hasGreeble(blah)
#endif

// Current compiler, short-circuit right-hand side
#if compiler(<10.0) || hasGreeble(blah)
#endif

// This compiler, don't short-circuit.
// expected-error@+1{{unexpected platform condition}}
#if compiler(>=5.7) && hasGreeble(blah)
#endif

// This compiler, don't short-circuit.
// expected-error@+1{{unexpected platform condition}}
#if compiler(<5.8) || hasGreeble(blah)
#endif

// Not a "version" check, so don't short-circuit.
// expected-error@+1{{unexpected platform condition}}
#if os(macOS) && hasGreeble(blah)
#endif

0 comments on commit 2da2561

Please # to comment.