Skip to content

[SR-2409] Rephrase diagnostic to consider Objective-C interop #8147

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 3 commits into from
Mar 16, 2017
Merged

[SR-2409] Rephrase diagnostic to consider Objective-C interop #8147

merged 3 commits into from
Mar 16, 2017

Conversation

d-ronnqvist
Copy link
Contributor

This PR picks up where rephrase diagnostic #4587 left off, looking at the value of EnableObjCInterop to choose between "C" and "Objective-C" as suggested by jrose-apple (in the conversation of that PR).

It updates one of the tests to check for different results on macOS and on Linux, similar to the 32/64-bit example in the Testing Swift document.

Resolves SR-2409.

When Objective-C interop is enabled, modules will continue to be referred to as “Objective-C modules”. But when interop is disabled, they will be referred to as “C modules”.

Updated the existing test for the current behavior that doesn’t expect to fail on linux and doesn’t require objc_interop, so that it checks to different diagnostics on the two systems.
@rintaro
Copy link
Member

rintaro commented Mar 16, 2017

@swift-ci Please smoke test

@d-ronnqvist
Copy link
Contributor Author

From the test output it looks like %target-os was "linux-gnu" making the check prefix expression:

--check-prefix=CHECK-linux-gnu

This is different than what's listed in the Testing Swift document, which is macosx, darwin, linux, freebsd, windows-cygnus, windows-gnu.

When swift-ci ran the tests, it looks like `%target-os` was "linux-gnu”, which wasn’t listed in the Testing Swift document (ony listing `macosx`, `darwin`, `linux`, `freebsd`, `windows-cygnus`, and `windows-gnu`)

Since I can’t (at least don’t know how to) run the linux tests myself, I’ll try and update the check-prefix to see if that passes.
@rintaro
Copy link
Member

rintaro commented Mar 16, 2017

Ah, could you use --check-prefix=CHECK-%target-runtime ?
It will be substituted by CHECK-objc(for macOS, iOS etc.) or CHECK-native (for Linux, etc.)

@d-ronnqvist
Copy link
Contributor Author

Yes. That's probably closer to what I had intended.

@rintaro
Copy link
Member

rintaro commented Mar 16, 2017

@swift-ci Please smoke test

1 similar comment
@rintaro
Copy link
Member

rintaro commented Mar 16, 2017

@swift-ci Please smoke test

@jrose-apple
Copy link
Contributor

Looks great. Thanks, David!

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