Skip to content

ClangImporter: improve ObjC-interop and enable tests #16275

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
May 2, 2018

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented May 1, 2018

This enables additional tests for the ClangImporter. This found a
missing piece in the -enable-objc-interop work that was done
previously. Address that and enable the tests. There are now two
failing tests on Linux:

  • sdk - depends on Foundation (not hermetic, see SR-7572)
  • import-mixed-with-header-twice - requires apple/swift PR#16022

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

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.

Looks good minus the one comment.

@@ -1,9 +1,8 @@
// RUN: %empty-directory(%t)
// RUN: not %target-swift-frontend -typecheck %s -I %S/Inputs/non-modular -F %S/Inputs/non-modular 2>&1 | %FileCheck --check-prefix=CHECK --check-prefix=CHECK-%target-runtime %s
// RUN: not %target-swift-frontend -enable-objc-interop -typecheck %s -I %S/Inputs/non-modular -F %S/Inputs/non-modular 2>&1 | %FileCheck --check-prefix=CHECK %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't change this test; we want to check both error messages. Or add a test with -disable-objc-interop instead.

Copy link
Member Author

@compnerd compnerd May 1, 2018

Choose a reason for hiding this comment

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

Sure, I'll add a second test case :-). We can test both paths on all targets now.

@compnerd compnerd force-pushed the clang-importer branch 2 times, most recently from 4f7a3f2 to 5915fdf Compare May 1, 2018 04:09
@compnerd
Copy link
Member Author

compnerd commented May 1, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented May 1, 2018

Build failed
Swift Test Linux Platform
Git Sha - 5915fdfa1675c1389ab0749224634d65ea215666

@swift-ci
Copy link
Contributor

swift-ci commented May 1, 2018

Build failed
Swift Test OS X Platform
Git Sha - 5915fdfa1675c1389ab0749224634d65ea215666

This enables additional tests for the ClangImporter.  This found a
missing piece in the `-enable-objc-interop` work that was done
previously.  Address that and enable the tests.  There are now the
following failing tests on Linux:

  * sdk - depends on Foundation (not hermetic, see SR-7572)
  * mixed-nsuinteger - depends on Foundation (not hermetic, see SR-7572)
  * import-mixed-with-header-twice - requires apple/swift PR#16022
  * can_import_objc_idempotent - requires apple/swift PR#16022
  * objc_protocol_renaming - requires apple/swift PR#16022
@compnerd
Copy link
Member Author

compnerd commented May 2, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented May 2, 2018

Build failed
Swift Test OS X Platform
Git Sha - 7606e2b

@swiftlang swiftlang deleted a comment from swift-ci May 2, 2018
@compnerd
Copy link
Member Author

compnerd commented May 2, 2018

@swift-ci please test macOS platform

@compnerd compnerd merged commit 46df353 into swiftlang:master May 2, 2018
@compnerd compnerd deleted the clang-importer branch May 2, 2018 20:03
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.

3 participants