Skip to content

#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

Merged
merged 4 commits into from
Nov 29, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,10 @@ ERROR(empty_version_string,none,
WARNING(unknown_platform_condition_argument,none,
"unknown %0 for build configuration '%1'",
(StringRef, StringRef))
WARNING(likely_simulator_platform_condition,none,
"plaform condition appears to be testing for simulator environment; "
"use 'targetEnvironment(simulator)' instead",
())

//------------------------------------------------------------------------------
// Availability query parsing diagnostics
Expand Down
4 changes: 3 additions & 1 deletion include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ namespace swift {
Runtime,
/// Conditional import of module
CanImport,
/// Target Environment (currently just 'simulator' or absent)
TargetEnvironment,
};

/// Describes which Swift 3 Objective-C inference warnings should be
Expand Down Expand Up @@ -351,7 +353,7 @@ namespace swift {
}

private:
llvm::SmallVector<std::pair<PlatformConditionKind, std::string>, 4>
llvm::SmallVector<std::pair<PlatformConditionKind, std::string>, 5>
PlatformConditionValues;
llvm::SmallVector<std::string, 2> CustomConditionalCompilationFlags;
};
Expand Down
15 changes: 15 additions & 0 deletions lib/Basic/LangOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
//===----------------------------------------------------------------------===//

#include "swift/Basic/LangOptions.h"
#include "swift/Basic/Platform.h"
#include "swift/Basic/Range.h"
#include "swift/Config.h"
#include "llvm/ADT/Hashing.h"
Expand Down Expand Up @@ -60,6 +61,10 @@ static const StringRef SupportedConditionalCompilationRuntimes[] = {
"_Native",
};

static const StringRef SupportedConditionalCompilationTargetEnvironments[] = {
"simulator",
};

template <size_t N>
bool contains(const StringRef (&Array)[N], const StringRef &V,
std::vector<StringRef> &suggestions) {
Expand Down Expand Up @@ -99,6 +104,9 @@ checkPlatformConditionSupported(PlatformConditionKind Kind, StringRef Value,
case PlatformConditionKind::Runtime:
return contains(SupportedConditionalCompilationRuntimes, Value,
suggestions);
case PlatformConditionKind::TargetEnvironment:
return contains(SupportedConditionalCompilationTargetEnvironments, Value,
suggestions);
case PlatformConditionKind::CanImport:
// All importable names are valid.
// FIXME: Perform some kind of validation of the string?
Expand Down Expand Up @@ -254,6 +262,13 @@ std::pair<bool, bool> LangOptions::setTarget(llvm::Triple triple) {
else
addPlatformConditionValue(PlatformConditionKind::Runtime, "_Native");

// Set the "targetEnvironment" platform condition if targeting a simulator
// environmet. Otherwise _no_ value is present for targetEnvironment; it's
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo environmet

// an optional disambiguating refinement of the triple.
if (swift::tripleIsAnySimulator(Target))
addPlatformConditionValue(PlatformConditionKind::TargetEnvironment,
"simulator");

// If you add anything to this list, change the default size of
// PlatformConditionValues to not require an extra allocation
// in the common case.
Expand Down
24 changes: 18 additions & 6 deletions lib/Basic/Platform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,37 @@ using namespace swift;
bool swift::tripleIsiOSSimulator(const llvm::Triple &triple) {
llvm::Triple::ArchType arch = triple.getArch();
return (triple.isiOS() &&
(arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64));
// FIXME: transitional, this should eventually stop testing arch, and
// switch to only checking the -environment field.
(triple.isSimulatorEnvironment() ||
arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64));
}

bool swift::tripleIsAppleTVSimulator(const llvm::Triple &triple) {
llvm::Triple::ArchType arch = triple.getArch();
return (triple.isTvOS() &&
(arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64));
// FIXME: transitional, this should eventually stop testing arch, and
// switch to only checking the -environment field.
(triple.isSimulatorEnvironment() ||
arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64));
}

bool swift::tripleIsWatchSimulator(const llvm::Triple &triple) {
llvm::Triple::ArchType arch = triple.getArch();
return (triple.isWatchOS() &&
(arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64));
// FIXME: transitional, this should eventually stop testing arch, and
// switch to only checking the -environment field.
(triple.isSimulatorEnvironment() ||
arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64));
}

bool swift::tripleIsAnySimulator(const llvm::Triple &triple) {
return tripleIsiOSSimulator(triple) ||
tripleIsWatchSimulator(triple) ||
tripleIsAppleTVSimulator(triple);
// FIXME: transitional, this should eventually just use the -environment
// field.
return triple.isSimulatorEnvironment() ||
tripleIsiOSSimulator(triple) ||
tripleIsWatchSimulator(triple) ||
tripleIsAppleTVSimulator(triple);
}

DarwinPlatformKind swift::getDarwinPlatformKind(const llvm::Triple &triple) {
Expand Down
99 changes: 99 additions & 0 deletions lib/Parse/ParseIfConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I just mistaken, or shouldn't this be a dyn_cast? cast implies that this must succeed and will abort if the expression type is not a UnresolvedDeclRefExpr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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(
Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 14 additions & 3 deletions test/Parse/ConditionalCompilation/identifierName.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ func f2(
FOO: Int,
swift: Int, _compiler_version: Int,
os: Int, arch: Int, _endian: Int, _runtime: Int,
targetEnvironment: Int,
arm: Int, i386: Int, macOS: Int, OSX: Int, Linux: Int,
big: Int, little: Int,
_ObjC: Int, _Native: Int
_ObjC: Int, _Native: Int,
simulator: Int
) {

#if FOO
Expand All @@ -23,6 +25,8 @@ func f2(
_ = _runtime + _ObjC + _Native
#elseif swift(>=1.0) && _compiler_version("3.*.0")
_ = swift + _compiler_version
#elseif targetEnvironment(simulator)
_ = targetEnvironment + simulator
#endif

}
Expand All @@ -31,9 +35,11 @@ func f2() {
let
FOO = 1, swift = 1, _compiler_version = 1,
os = 1, arch = 1, _endian = 1, _runtime = 1,
targetEnvironment = 1,
arm = 1, i386 = 1, macOS = 1, OSX = 1, Linux = 1,
big = 1, little = 1,
_ObjC = 1, _Native = 1
_ObjC = 1, _Native = 1,
simulator = 1

#if FOO
_ = FOO
Expand All @@ -47,6 +53,8 @@ func f2() {
_ = _runtime + _ObjC + _Native
#elseif swift(>=1.0) && _compiler_version("3.*.0")
_ = swift + _compiler_version
#elseif targetEnvironment(simulator)
_ = targetEnvironment + simulator
#endif

}
Expand All @@ -55,16 +63,19 @@ struct S {
let
FOO = 1, swift = 1, _compiler_version = 1,
os = 1, arch = 1, _endian = 1, _runtime = 1,
targetEnvironment = 1,
arm = 1, i386 = 1, macOS = 1, OSX = 1, Linux = 1,
big = 1, little = 1,
_ObjC = 1, _Native = 1
_ObjC = 1, _Native = 1,
simulator = 1

#if FOO
#elseif os(macOS) && os(OSX) && os(Linux)
#elseif arch(i386) && arch(arm)
#elseif _endian(big) && _endian(little)
#elseif _runtime(_ObjC) && _runtime(_Native)
#elseif swift(>=1.0) && _compiler_version("3.*.0")
#elseif targetEnvironment(simulator)
#endif

}
Expand Down
39 changes: 39 additions & 0 deletions test/Parse/ConditionalCompilation/simulatorTargetEnv.swift
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