Skip to content

[SILOptimizer]Refactor ObjC dependencies in CSE tests #832

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 1 commit into from
Dec 31, 2015
Merged

[SILOptimizer]Refactor ObjC dependencies in CSE tests #832

merged 1 commit into from
Dec 31, 2015

Conversation

ezephir
Copy link

@ezephir ezephir commented Dec 31, 2015

This change moves functionality that requires ObjC interop into a new file and removes the XFAIL on Linux. Partially resolves SR-216.

This change was tested both on Ubuntu 14.04 and OS X 10.10.

Question for the review:

Should tests involving @objc be moved out, even though they pass on Linux? (See the first few tests on line 740 or the first few tests on line 944 as examples of what I mean.

Thank you!

This change moves tests that require ObjC into a new file and removes the XFAIL
on Linux. Partially resolves SR-216.
@ezephir
Copy link
Author

ezephir commented Dec 31, 2015

I also guess its worth asking:

I tried to use a #if _runtime(_ObjC) build configuration on Linux but the guarded SIL code still failed. This appears to work OK for Swift files and in the REPL.

  • Is this something that should work?
  • Would you prefer me to use this instead of splitting out tests?

gribozavr added a commit that referenced this pull request Dec 31, 2015
[SILOptimizer]Refactor ObjC dependencies in CSE tests
@gribozavr gribozavr merged commit c27ffd2 into swiftlang:master Dec 31, 2015
@gribozavr
Copy link
Contributor

@ezephir Thank you! Splitting out tests is preferred for now.

@ezephir ezephir deleted the linux-test-cse branch January 1, 2016 09:42
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.

2 participants