Add support for textual imports to -emit-objc-header

[NuriAmari]

A second attempt at #60971, but without the flaky test.

The test that was removed was designed to test header emission against the SDK. It was already complicated having to handle the SDK on different platforms, and would break if the SDK changed. Rather than attempt to make a test that is vague enough to be SDK resilient I've just removed the test; the tests against the more stable mock-sdk should be enough to prevent regression. The functionality itself has been tested extensively against the SDK and other libraries.

[NuriAmari]

@swift-ci Please test

[hyp]

@swift-ci please test source compatibility

[etcwilde]

This looks fine to me. We'll have to keep out eye on https://ci.swift.org/job/oss-swift_tools-RA_stdlib-DA_test-device-non_executable to make sure it lands cleanly.

[ncooke3]

Hi @NuriAmari–– I'm working on adding Swift Package Manager support for mixed language (Swift + Objective-C) targets in swiftlang/swift-package-manager#5919. I'm running into an issue and am unsure if this PR's flag is a solution to the same problem– I was hoping you could weigh in. The -emit-objc-header generates a header for Objective-C compatible Swift API that will contain a module style import for symbols that can't be forward declared. The issue is that this import assumes a framework directory structure and the presence of an umbrella header. It'd be nice if mixed SPM targets were not bound by this requirement.

Based on my understanding of this PR, the -emit-clang-header-nonmodular-includes might be the perfect solution here? Instead of the module style import, I'm assuming that the target's public headers would be scanned and imported with non-modular import statements?

To test, I checked out the nightly toolchain and passed this flag instead of -emit-objc-header when building the Swift part of a mixed target.

The problem I'm facing now is that, although I noticed the generated interop header change slightly, the new flag does not prevent the addition of the module style umbrella import.

I'll share the relevant part of my generated header:

• With the current -emit-objc-header:

  #if defined(__OBJC__)
  #if __has_feature(modules)
  #if __has_warning("-Watimport-in-framework-header")
  #pragma clang diagnostic ignored "-Watimport-in-framework-header"
  #endif
  #endif
  
  #import <BasicMixedTarget/BasicMixedTarget.h> 💥 This import is problematic for SPM targets

• With the new -emit-clang-header-nonmodular-includes:

  #if defined(__OBJC__)
  #if __has_feature(objc_modules)
  #if __has_warning("-Watimport-in-framework-header")
  #pragma clang diagnostic ignored "-Watimport-in-framework-header"
  #endif
  #else 💥 This `else` clause is new
  #endif
  
  #import <BasicMixedTarget/BasicMixedTarget.h> 💥 This import is still here, which is problematic

• What I'm hoping to see:

  #if defined(__OBJC__)
  #if __has_feature(objc_modules)
  #if __has_warning("-Watimport-in-framework-header")
  #pragma clang diagnostic ignored "-Watimport-in-framework-header"
  #endif
  #else
  #import "path/to/public/header/from/target.h"   💥 I was hoping for something like this (for each public header in the target)
  #endif
  
  💥 The module style import is gone.

Note: I also used -emit-objc-header-path in combo with both of the above flags to point to where I want this header to be generated at.

I'm now thinking that this new flag has no effect on the #import <BasicMixedTarget/BasicMixedTarget.h>– am I correct?

I'd appreciate any thoughts!

[NuriAmari]

Hi @ncooke3

Unfortunately I don't think this flag will have the effect you are looking for.

Instead of the module style import, I'm assuming that the target's public headers would be scanned and imported with non-modular import statements?

Sort of, yes it translates to non-modular imports, but if there are umbrella headers available it will use them. You can read the change to see more detail, but the logic goes something like:

-> If the Clang module has an umbrella header, include that
-> Otherwise, if the Clang module has an umbrella directory, include all the headers in there
-> Otherwise, just include all the public headers listed in the Clang module

But these headers are only included if the Clang module definition specifies them, which I'm assuming wouldn't be an issue for you.

I'm now thinking that this new flag has no effect on the #import <BasicMixedTarget/BasicMixedTarget.h>– am I correct?

Yes, this change has no effect on that include. It is generated here: https://github.com/NuriAmari/swift/blob/cd4b9a64169550a47b795c86f5d9fa0d2376b0c9/lib/PrintAsClang/PrintAsClang.cpp#L636-L643 . If the code is any indication, that include is meant to be a bridging header.

The issue is that this import assumes a framework directory structure and the presence of an umbrella header.

Do you mean by including #import <BasicMixedTarget/BasicMixedTarget.h> it assumes this, or does it somehow else assume Framework structure. Is your issue actually ever with the modular imports, or just this textual one?

Happy to help, I think what you want is achievable, but I'm not entirely sure what that is right now.

[ncooke3]

Thanks @NuriAmari for taking the time- this is helpful!

Do you mean by including #import <BasicMixedTarget/BasicMixedTarget.h> it assumes this, or does it somehow else assume Framework structure.

Yes, I'm claiming that by including #import <BasicMixedTarget/BasicMixedTarget.h>, the compiler's generation assumes a framework structure. I say this because, based on my understanding of <...> style imports, the path within <BasicMixedTarget/BasicMixedTarget.h> is resolved based on a framework directory structure like this:

MixedTarget.framework
└── Headers
    ├── ...
    └── MixedTarget.h

I have a feeling I'm wrong about this though–– I'm not super confident about my understanding of "..." vs. <...> style imports and how the latter is resolved. So please do correct me if I'm wrong here.

Is your issue actually ever with the modular imports, or just this textual one?

If by textual one, you are referring to #import <BasicMixedTarget/BasicMixedTarget.h>, then the answer is just the textual one!

Unless a mixed target (e.g. BasicMixedTarget) contains BasicMixedTarget/BasicMixedTarget.h within the public headers directory, the build fails because the problematic textual import in the -Swift.h can't be resolved. This makes sense to me, but I'm trying to avoid passing this constraint onto package authors.

My current workaround I've been locally developing/testing does the following:

• Check if the root of the target's public header directory contains subdirectory (named after the target -- e.g. $(TargetName)/) that includes an umbrella header (e.g. $(TargetName).h).
  • If it does, do nothing because the problematic textual import within the -Swift.h interop header seems to ignore the brackets and treat the #import <BasicMixedTarget/BasicMixedTarget.h> import as #import "BasicMixedTarget/BasicMixedTarget.h"–– which is successfully resolved since we pass the target's public headers directory as a header search path via -I.
  • Else, generate an umbrella header that imports all public .h headers and use a VFS overlay during compilation to make the generated umbrella header appear that it is within a subdirectory (named after the module name) at the root of the public header directory.

This workaround seems to work. One side effect is that the target's sources as well as clients can import the generated umbrella header– which might seem a bit unexpected because it is a build artifact. On the other hand, the target's sources can import the generated Swift header so there is some precedent for importing generated build artifacts.

In the case of the BasicMixedTarget example, I'm trying to not force the target's author to have to include BasicMixedTarget/BasicMixedTarget.h within the public headers directory.

Based on the code you sent, what would be better than my current workaround would be it there was a flag to pass in the bridging header imported within the -Swift.h file.

I'm very much open to any alternative ideas as well!

[ncooke3]

Based on the code you sent, what would be better than my current workaround would be it there was a flag to pass in the bridging header imported within the -Swift.h file.

I worked backwards from the code you sent and found the -import-objc-header <path> argument to swiftc. That seems to allow you to pass the bridging header which might be the thing I'm looking for. Interestingly, I don't see it when listed when running swiftc --help. I will investigate this tomorrow.

[NuriAmari]

I have a feeling I'm wrong about this though–– I'm not super confident about my understanding of "..." vs. <...> style imports and how the latter is resolved. So please do correct me if I'm wrong here.

Generally in C based languages, <...> is for system headers, "..." is for user created headers, but to my knowledge <...> still respects -I arguments. I would have to read the Clang source to find the exact difference in behavior, I don't know off the top of my head.

If by textual one, you are referring to #import <BasicMixedTarget/BasicMixedTarget.h>, then the answer is just the textual one!

Yes by textual I just mean #import not @import. The point of this change is to transform a set of @import statements to an equivalent set of #import statements.

I worked backwards from the code you sent and found the -import-objc-header argument to swiftc. That seems to allow you to pass the bridging header which might be the thing I'm looking for. Interestingly, I don't see it when listed when running swiftc --help. I will investigate this tomorrow.

Certainly worth a try. Use swiftc --help-hidden and swiftc -frontend --help-hidden for more flags. There is also -bridging-header-directory-for-print that seems to let you prefix the header include path with some other path in case that's helpful.

[ncooke3]

Thanks @NuriAmari!

I experimented with both options but couldn't get the textual import to change. Based on this code, it seems that in order to use the value from the -bridging-header-directory-for-print option, I need to also use -import-objc-header.

Unfortunately though, I cannot use -import-objc-header because an error is thrown when trying to use it in tandem with -import-underlying-module.

I think this limitation makes sense to me that you can either use one or the other. So far, -import-underlying-module has been the magical flag to expose ObjC headers to the Swift part of the target. Maybe I can drop -import-underlying-module and adjust -import-objc-header to make it do the same thing... thus far, -import-underlying-module uses a generated module map that exposes all headers in the target via an umbrella directory–– I think I could maybe achieve the same thing with by instead generating an umbrella header and passing it to -import-objc-header.

---

Edit:

I think I could maybe achieve the same thing with by instead generating an umbrella header and passing it to -import-objc-header.

That didn't seem to work. The Swift sources can't seem to find any ObjC types within the scope. I think I'll have to stick to the workaround described in my above comment #62386 (comment).