[ClangImporter] Can't correct import closure that returns NSString

[DataCorrupted]

No description provided.

[compnerd]

Do we need ObjC interop? Can we not simply create a header that declares @class NSString and use a module to import that?

[DataCorrupted]

If I remove objc_interop and change #import <Foundation/Foundation.h> to @class NSString;, the file triggers a type check error (shown below) instead of crashing SILGen.

/Users/peterrong/swift-project/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/test-macosx-arm64/ClangImporter/Output/objc_method_with_NSString_clousure.swift.tmp/Theme.swift:2:16: error: closure passed to parameter of type 'OpaquePointer' that does not accept a closure
1 | let _ = ThemeFuncTable(
2 |     keyGetter: { "1" }
  |                `- error: closure passed to parameter of type 'OpaquePointer' that does not accept a closure
3 | )
4 | 

/Users/peterrong/swift-project/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/test-macosx-arm64/ClangImporter/Output/objc_method_with_NSString_clousure.swift.tmp/Theme.h:2:16: note: 'init(keyGetter:)' declared here
1 | @class NSString;
2 | typedef struct ThemeFuncTable {
  |                `- note: 'init(keyGetter:)' declared here
3 |   NSString *_Nonnull (*_Nonnull keyGetter)();
4 | } ThemeFuncTable;

[compnerd]

You will likely need to pass -enable-objc-interop to the %target-swift-frontend when you do that.

[DataCorrupted]

Just tried. The type check error is the same. I assume this crash can't be triggered through @class NSString and has to #import <Foundation/Foundation.h>.

[compnerd]

That doesn't make any sense - #import <Foundation/Foundation.h> still creates a clang module that is imported the same as here. The declaration is different. Perhaps you need something a bit more complete? Something like:

@interface NSString: NSObject
@end

[DataCorrupted]

Using this, swift side complains it can't convert String into NSString, and the point of this bug is incorrect type conversion, so I guess some bridging code in Foundation matters?

[compnerd]

I don't know the declarations off the top of my head, you should see what the declaration for NSString is.

[DataCorrupted]

Actually @CodaFi 's suggestion worked. Kindly resolve the comment if that works for you.

[CodaFi]

Nit: objc_method_with_NSString_closure.swift

[CodaFi]

There's also no CHECK lines in this test so it's going to fail. Consider

// RUN: not --crash %target-swift-frontend(mock-sdk: %clang-importer-sdk) -emit-silgen %t/Theme.swift -import-objc-header %t/Theme.h

instead

[DataCorrupted]

It works, thanks. Now the code crashes on both my Mac and linux server.

However, I'm not sure we want to merge it by putting not --crash there without a fix. I wonder if anyone has any insight on why there is a crash.