-
Notifications
You must be signed in to change notification settings - Fork 10.5k
#if targetEnvironment(simulator) #12964
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
3807b3f
d3613b8
d31bad4
94988a2
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 |
---|---|---|
|
@@ -37,6 +37,7 @@ Optional<PlatformConditionKind> getPlatformConditionKind(StringRef Name) { | |
.Case("_endian", PlatformConditionKind::Endianness) | ||
.Case("_runtime", PlatformConditionKind::Runtime) | ||
.Case("canImport", PlatformConditionKind::CanImport) | ||
.Case("targetEnvironment", PlatformConditionKind::TargetEnvironment) | ||
.Default(None); | ||
} | ||
|
||
|
@@ -325,6 +326,8 @@ class ValidateIfConfigCondition : | |
DiagName = "endianness"; break; | ||
case PlatformConditionKind::CanImport: | ||
DiagName = "import conditional"; break; | ||
case PlatformConditionKind::TargetEnvironment: | ||
DiagName = "target environment"; break; | ||
case PlatformConditionKind::Runtime: | ||
llvm_unreachable("handled above"); | ||
} | ||
|
@@ -544,8 +547,97 @@ static bool isVersionIfConfigCondition(Expr *Condition) { | |
return IsVersionIfConfigCondition().visit(Condition); | ||
} | ||
|
||
/// Get the identifier string from an \c Expr if it's an | ||
/// \c UnresolvedDeclRefExpr, otherwise the empty string. | ||
static StringRef getDeclRefStr(Expr *E) { | ||
if (auto *UDRE = cast<UnresolvedDeclRefExpr>(E)) { | ||
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. Am I just mistaken, or shouldn't this be a 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. Oh, yes, this is a typo, thanks! Will fix tomorrow. |
||
return UDRE->getName().getBaseIdentifier().str(); | ||
} | ||
return ""; | ||
} | ||
|
||
static bool isPlatformConditionDisjunction(Expr *E, PlatformConditionKind Kind, | ||
ArrayRef<StringRef> Vals) { | ||
if (auto *Or = dyn_cast<BinaryExpr>(E)) { | ||
if (getDeclRefStr(Or->getFn()) == "||") { | ||
auto Args = Or->getArg()->getElements(); | ||
return (isPlatformConditionDisjunction(Args[0], Kind, Vals) && | ||
isPlatformConditionDisjunction(Args[1], Kind, Vals)); | ||
} | ||
} else if (auto *P = dyn_cast<ParenExpr>(E)) { | ||
return isPlatformConditionDisjunction(P->getSubExpr(), Kind, Vals); | ||
} else if (auto *C = dyn_cast<CallExpr>(E)) { | ||
if (getPlatformConditionKind(getDeclRefStr(C->getFn())) != Kind) | ||
return false; | ||
if (auto *ArgP = dyn_cast<ParenExpr>(C->getArg())) { | ||
if (auto *Arg = ArgP->getSubExpr()) { | ||
auto ArgStr = getDeclRefStr(Arg); | ||
for (auto V : Vals) { | ||
if (ArgStr == V) | ||
return true; | ||
} | ||
} | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
// Search for the first occurrence of a _likely_ (but not definite) implicit | ||
// simulator-environment platform condition, or negation thereof. This is | ||
// defined as any logical conjunction of one or more os() platform conditions | ||
// _strictly_ from the set {iOS, tvOS, watchOS} and one or more arch() platform | ||
// conditions _strictly_ from the set {i386, x86_64}. | ||
// | ||
// These are (at the time of writing) defined as de-facto simulators in | ||
// Platform.cpp, and if a user is testing them they're _likely_ looking for | ||
// simulator-ness indirectly. If there is anything else in the condition aside | ||
// from these conditions (or the negation of such a conjunction), we | ||
// conservatively assume the user is testing something other than | ||
// simulator-ness. | ||
static Expr *findAnyLikelySimulatorEnvironmentTest(Expr *Condition) { | ||
|
||
if (!Condition) | ||
return nullptr; | ||
|
||
if (auto *N = dyn_cast<PrefixUnaryExpr>(Condition)) { | ||
return findAnyLikelySimulatorEnvironmentTest(N->getArg()); | ||
} else if (auto *P = dyn_cast<ParenExpr>(Condition)) { | ||
return findAnyLikelySimulatorEnvironmentTest(P->getSubExpr()); | ||
} | ||
|
||
// We assume the user is writing the condition in CNF -- say (os(iOS) || | ||
// os(tvOS)) && (arch(i386) || arch(x86_64)) -- rather than DNF, as the former | ||
// is exponentially more terse, and these conditions are already quite | ||
// unwieldy. If field evidence shows people using other variants, possibly add | ||
// them here. | ||
|
||
auto isSimulatorPlatformOSTest = [](Expr *E) -> bool { | ||
return isPlatformConditionDisjunction( | ||
E, PlatformConditionKind::OS, {"iOS", "tvOS", "watchOS"}); | ||
}; | ||
|
||
auto isSimulatorPlatformArchTest = [](Expr *E) -> bool { | ||
return isPlatformConditionDisjunction( | ||
E, PlatformConditionKind::Arch, {"i386", "x86_64"}); | ||
}; | ||
|
||
if (auto *And = dyn_cast<BinaryExpr>(Condition)) { | ||
if (getDeclRefStr(And->getFn()) == "&&") { | ||
auto Args = And->getArg()->getElements(); | ||
if ((isSimulatorPlatformOSTest(Args[0]) && | ||
isSimulatorPlatformArchTest(Args[1])) || | ||
(isSimulatorPlatformOSTest(Args[1]) && | ||
isSimulatorPlatformArchTest(Args[0]))) { | ||
return And; | ||
} | ||
} | ||
} | ||
return nullptr; | ||
} | ||
|
||
} // end anonymous namespace | ||
|
||
|
||
/// Parse and populate a #if ... #endif directive. | ||
/// Delegate callback function to parse elements in the blocks. | ||
ParserResult<IfConfigDecl> Parser::parseIfConfig( | ||
|
@@ -594,6 +686,13 @@ ParserResult<IfConfigDecl> Parser::parseIfConfig( | |
diag::extra_tokens_conditional_compilation_directive); | ||
} | ||
|
||
if (Expr *Test = findAnyLikelySimulatorEnvironmentTest(Condition)) { | ||
diagnose(Test->getLoc(), | ||
diag::likely_simulator_platform_condition) | ||
.fixItReplace(Test->getSourceRange(), | ||
"targetEnvironment(simulator)"); | ||
} | ||
|
||
// Parse elements | ||
SmallVector<ASTNode, 16> Elements; | ||
if (isActive || !isVersionCondition) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// RUN: %swift -swift-version 4 -typecheck %s -verify -target x86_64-apple-ios7.0 -parse-stdlib | ||
// RUN: %swift -swift-version 4 -typecheck %s -verify -target x86_64-unknown-linux-simulator -parse-stdlib | ||
// RUN: %swift-ide-test -swift-version 4 -test-input-complete -source-filename=%s -target x86_64-apple-ios7.0 | ||
|
||
#if !targetEnvironment(simulator) | ||
// This block should not parse. | ||
let i: Int = "Hello" | ||
#endif | ||
|
||
#if targetEnvironment(simulator) | ||
class C {} | ||
var x = C() | ||
#endif | ||
var y = x | ||
|
||
#if os(iOS) && arch(i386) | ||
// expected-warning @-1 {{plaform condition appears to be testing for simulator environment}} {{5-26=targetEnvironment(simulator)}} | ||
class C1 {} | ||
#endif | ||
|
||
#if arch(i386) && os(iOS) | ||
// expected-warning @-1 {{plaform condition appears to be testing for simulator environment}} {{5-26=targetEnvironment(simulator)}} | ||
class C2 {} | ||
#endif | ||
|
||
#if arch(i386) && (os(iOS) || os(watchOS)) | ||
// expected-warning @-1 {{plaform condition appears to be testing for simulator environment}} {{5-43=targetEnvironment(simulator)}} | ||
class C3 {} | ||
#endif | ||
|
||
#if (arch(x86_64) || arch(i386)) && (os(iOS) || os(watchOS) || os(tvOS)) | ||
// expected-warning @-1 {{plaform condition appears to be testing for simulator environment}} {{5-73=targetEnvironment(simulator)}} | ||
class C4 {} | ||
#endif | ||
|
||
#if !(arch(x86_64) && os(tvOS)) | ||
// expected-warning @-1 {{plaform condition appears to be testing for simulator environment}} {{7-31=targetEnvironment(simulator)}} | ||
class C5 {} | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo
environmet