Skip to content

New compiler directive to replace _compiler_version #15977

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 5, 2018
Merged

New compiler directive to replace _compiler_version #15977

merged 2 commits into from
Jun 5, 2018

Conversation

hartbit
Copy link
Contributor

@hartbit hartbit commented Apr 17, 2018 •
edited
Loading

This is implementation for the Compiler Version Directive proposal: swiftlang/swift-evolution#833

@hartbit hartbit mentioned this pull request Apr 17, 2018
Merged
@jrose-apple jrose-apple added the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Apr 17, 2018
@hartbit hartbit force-pushed the compiler-directive branch 2 times, most recently from 40a3317 to 8079cbf Compare April 18, 2018 08:50
@tkremenek
Copy link
Member

The implementation also needs to cover this case that came up during the review evolution thread.

@hartbit hartbit force-pushed the compiler-directive branch from 8079cbf to 6e51997 Compare May 18, 2018 20:57
@hartbit hartbit changed the title [WIP] New compiler directive to replace _compiler_version New compiler directive to replace _compiler_version May 18, 2018
@hartbit
Copy link
Contributor Author

hartbit commented May 18, 2018

@tkremenek Isn't that already the case in Version::getEffectiveLanguageVersion()? Can you point me as to how the implementation is incorrect?

@hartbit
Copy link
Contributor Author

hartbit commented May 31, 2018

@tkremenek Any news?

@tkremenek
Copy link
Member

@swift-ci test

@tkremenek
Copy link
Member

@hartbit you're absolutely right. Sorry for the delay in responding.

@jrose-apple can you give this review with another pair of eyes? Looks fairly straightforward.

@tkremenek tkremenek requested a review from jrose-apple May 31, 2018 13:41
@tkremenek tkremenek removed the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label May 31, 2018
@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 8079cbf55a456165b2a93e24398fc65dd6fccdc9

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 8079cbf55a456165b2a93e24398fc65dd6fccdc9

jrose-apple
jrose-apple approved these changes May 31, 2018
View reviewed changes
Copy link
Contributor

@jrose-apple jrose-apple left a comment

Choose a reason for hiding this comment

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

It looks good to me, with one potential cleanup comment.

lib/Parse/ParseIfConfig.cpp Outdated
auto Val = version::Version::parseVersionString(
Str, SourceLoc(), nullptr).getValue();
auto thisVersion = version::Version::getCurrentLanguageVersion();
return thisVersion >= Val;
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to share code with the "swift" case above!

@hartbit hartbit force-pushed the compiler-directive branch from 6e51997 to 2abb931 Compare June 1, 2018 20:15
@hartbit
Copy link
Contributor Author

hartbit commented Jun 1, 2018

@swift-ci test

@swift-ci
Copy link
Contributor

swift-ci commented Jun 1, 2018

Build failed
Swift Test OS X Platform
Git Sha - 6e51997628b97cc7b709c496b2ae79f1a934226e

@hartbit
Copy link
Contributor Author

hartbit commented Jun 1, 2018

@jrose-apple I addressed your cleanup comment. Thanks for the review!

@swift-ci
Copy link
Contributor

swift-ci commented Jun 1, 2018

Build failed
Swift Test Linux Platform
Git Sha - 6e51997628b97cc7b709c496b2ae79f1a934226e

@tkremenek
Copy link
Member

@swift-ci smoke test

@tkremenek tkremenek merged commit 81658f4 into swiftlang:master Jun 5, 2018
@tkremenek
Copy link
Member

tkremenek commented Jun 5, 2018 •
edited
Loading

@hartbit can you prepare another pull request against swift-4.2-branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrose-apple jrose-apple jrose-apple approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hartbit @tkremenek @swift-ci @jrose-apple