Skip to content
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

#if targetEnvironment(simulator) #12964

Merged
merged 4 commits into from
Nov 29, 2017

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Nov 16, 2017

This is an implementation of swiftlang/swift-evolution#764 which has now been accepted. It is ready to merge.

@graydon
Copy link
Contributor Author

graydon commented Nov 16, 2017

@swift-ci please test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

This looks fantastic, thank you @graydon

@@ -1503,6 +1503,9 @@ 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",
Copy link
Member

Choose a reason for hiding this comment

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

Could you tack on "; use 'targetEnvironment(simulator)' instead" ? That way, one doesn't have to look at the contents of the Fix-It to see what the fix is.

@@ -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

@graydon graydon changed the title [DO NOT MERGE] #if targetEnvironment(simulator) #if targetEnvironment(simulator) Nov 28, 2017
@graydon
Copy link
Contributor Author

graydon commented Nov 28, 2017

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 81cc50b181e2874816d0c1f7eac3bb69b70c417e

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 81cc50b181e2874816d0c1f7eac3bb69b70c417e

@tkremenek tkremenek merged commit e04c7fd into swiftlang:master Nov 29, 2017
/// 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.

graydon added a commit to graydon/swift that referenced this pull request Nov 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants