PrintAsClang: Print a C block in the compatibility header for `@cdecl` functions #80917

xymus · 2025-04-18T19:01:28Z

Add a block for C clients in the compatibility header. This block contains only the C representation of the @cdecl functions that are printed using only C types.

This C block is printed above the Objective-C and C++ blocks as if we add support for @cdecl types other languages should be able to reference them in function signatures. Other languages block don't duplicate printing the @cdecl functions either as they are already accessible to them.

Nullability attributes may still be printed so we simply ignore them for C clients.

xymus · 2025-04-18T19:01:40Z

@swift-ci Please smoke test

beccadax · 2025-04-18T19:10:02Z

Before I dig into the details: What's with the nullability stuff here? I mean, obviously you want to ensure that compilers which don't support nullability annotations can still understand the header, but it seems like you're disabling (at least some parts of) nullability checking even in compilers that support it. Why?

xymus · 2025-04-18T19:41:25Z

In C mode I don't think clang supports nullability checking. It recognizes _Nonnull and raises warning: type nullability specifier '_Nonnull' is a Clang extension [-Werror,-Wnullability-extension]. I don't see a way to enable this extension for C as it's gated on Objective-C (without modifying clang at least). So instead here we disable the warning in compilers that support the attribute so it's ignored for C clients.

I don't like it so much either tbh. Alternatively we could change the printing logic to simply not print those attributes in the C block, but I'm thinking these attributes may still be useful for Objective-C clients calling a @cdecl function. In a more practical way to avoid side effects I'm also considering restricting the current changes under Ignore nullability attributes for C clients only to the C block.

xymus · 2025-04-18T20:11:40Z

@swift-ci Please smoke test

beccadax

Good start, but I have a few concerns.

beccadax · 2025-04-18T21:06:39Z

lib/PrintAsClang/PrintAsClang.cpp

+  // Ignore nullability attributes for C clients.
+  out << "#if !defined(__OBJC__) && !defined(__cplusplus)\n"
+         "# if __has_feature(nullability)\n"
+         "#  pragma clang diagnostic ignored \"-Wnullability-extension\"\n"
+         "# else\n"
+         "#  define _Nonnull\n"
+         "# endif\n"
+         "#endif\n";


There are a few issues I’d address here:

The comment doesn’t quite reflect what you’re actually doing here (which is why I asked about the -W flag; it didn’t match the comment).

It looks to me like only ObjC mode prevents the -Wnullability-extension warning from being emitted—C++ mode doesn't.

I believe there are circumstances where we’ll emit _Nullable, _Null_unspecified, and possibly _Nullable_result in addition to _Nonnull.

The upshot is, consider doing this instead:

Suggested change

// Ignore nullability attributes for C clients.

out << "#if !defined(__OBJC__) && !defined(__cplusplus)\n"

"# if __has_feature(nullability)\n"

"# pragma clang diagnostic ignored \"-Wnullability-extension\"\n"

"# else\n"

"# define _Nonnull\n"

"# endif\n"

"#endif\n";

// For C compilers which don’t support nullability attributes, ignore them;

// for ones which do, suppress warnings about them being an extension.

out << "#if !__has_feature(nullability)\n"

"# define _Nonnull\n"

"# define _Nullable\n"

"# define _Null_unspecified\n"

"#elif !defined(__OBJC__)\n"

"# pragma clang diagnostic ignored \"-Wnullability-extension\"\n"

"#endif\n"

"#if !__has_feature(nullability_nullable_result)\n"

"# define _Nullable_result _Nullable\n"

"#endif\n";

However, I do have a lingering concern about even this version: the C standard reserves underscore-uppercase identifiers for the compiler and standard library to use, so I believe #define-ing _Nonnull and friends is technically not valid. I might run this by some clang engineers to see if they recommend you do something different.

I applied this change and gated it behind the feature flag for now. I'll look deeper into types representable in C next and should be able to clean up the story around nullability then.

beccadax · 2025-04-18T21:13:44Z

test/PrintAsObjC/cdecl-official.swift

+
+@cdecl("has_keyword_arg_names")
+func c_keywordArgNames(auto: Int, union: Int) {}
+// CHECK-LABEL: SWIFT_EXTERN void has_keyword_arg_names(ptrdiff_t auto_, ptrdiff_t union_) SWIFT_NOEXCEPT;


I would add test cases for various pointer types: UnsafeMutableRawPointer should become void * _Nonnull, UnsafeMutablePointer<Int> should become ptrdiff_t * _Nonnull, the non-Mutable variants should gain a const, and adding ? or ! at the end should get you _Nullable and _Null_unspecified respectively. (Unless we’re in an assume_nonnull region? If you think so, there should be a CHECK line for that.)

Good point, I'm taking note of these. I plan on adding more test and type-checking around the types accepted by @cdecl in a future PR, a few more are not covered either.

We already get the desired behavior so I've added tests for pointer types and nullability to this PR.

beccadax · 2025-04-18T21:25:00Z

lib/PrintAsClang/PrintAsClang.cpp

+
+  // C content (@cdecl)
+  if (M->getASTContext().LangOpts.hasFeature(Feature::CDecl)) {
+    SmallPtrSet<ImportModuleTy, 8> imports;


When you’re in C++ or ObjC mode, are there going to be two sets of imports? Do both of them actually get printed? What if there’s overlap between them?

I kept the logic collecting imports as part of the boilerplate when I set up the C block but not the imports printing part. I don't expect it to collect anything at this point and it's unclear to me if we'll need to print imports for C in the future. Maybe for imported @cdecl enums or eventually some form of @cdecl struct, but even then we may have to print a local copy as we wouldn't know what header to import. Do you see other scenarios where we'd need imports for C?

Imports come up when a declaration uses a type from another (C) library. For instance, suppose you have this header:

// CoreToaster/CTTimeSetting.h typedef enum __attribute__((enum_extensibility(open)) CTTimeSetting CTTimeSetting; enum CTTimeSetting { CTTimeSettingLight, CTTimeSettingNormal, CTTimeSettingDark };

And you write this Swift file:

// ToasterKit.swift import CoreToaster @cdecl(TKGetDefaultTimeSetting) public func defaultTimeSetting() -> CTTimeSetting { .normal }

The ToasterKit generated header will need to use CTTimeSetting, which means it has to import the header it came from:

// ToasterKit+Swift.h #include <CoreToaster/CTTimeSetting.h> // ...other stuff... CTTimeSetting TKGetDefaultTimeSetting();

That makes perfect sense. I've added imported types to the pitch already and plan on adding support for C includes in a future PR.

beccadax · 2025-04-18T21:27:54Z

test/PrintAsObjC/cdecl-official.swift

+
+//--- Lib.swift
+
+// CHECK: #if defined(__cplusplus)


Strongly recommend adding test cases that require an import and then testing the imports you get.

Remember, to be compatible with non-clang compilers, official cdecl needs to not use @import; it needs to use #include instead (or at least fall back to it). It also needs to not use the .h-less #includes that are common in C++.

Yeah, C includes with require some new logic to the printing component.

Add a block for C clients in the compatibility header. This block contains only the `@cdecl` functions that are printed using only C types. This C block is printed above the Objective-C and C++ blocks as if we add support for `@cdecl` types other languages should be able to reference them in function signatures. Other languages block don't duplicate printing the `@cdecl` functions either as they are already accessible to them.

In C mode we still print nullability attributes. Don't let clang warn on them and ignore the attribute if the C compiler doesn't know them.

xymus · 2025-04-18T22:33:53Z

@swift-ci Please smoke test

lib/PrintAsClang/DeclAndTypePrinter.cpp

Xazax-hun · 2025-04-22T10:20:02Z

lib/PrintAsClang/DeclAndTypePrinter.cpp

@@ -2999,6 +3006,14 @@ bool DeclAndTypePrinter::shouldInclude(const ValueDecl *VD) {
      return false;
  }

+  // In C output mode print only @cdecls and skip them in other modes.


Could you elaborate on this? Where did we emit cdecls before this change?

What if someone what to consume these cdecls from ObjC or C++ code? Is that still possible?

We print the new C specific @cdecl only in C mode and keep printing the classic @_cdecl in other modes. The other modes print @_cdecl in they own block of the compatibility header gated behind the corresponding language check. I've updated the comment clarify this.

Objective-C and C++ clients see and can call the newly printed @cdecl functions. They are printed in a block at the beginning of the compatibility header without a language check except for an extern "C" block.

xymus · 2025-04-30T18:24:59Z

@swift-ci Please smoke test Linux

xymus · 2025-04-30T18:25:08Z

@swift-ci Please smoke test Windows

xymus · 2025-05-05T18:49:48Z

@swift-ci Please smoke test

xymus · 2025-05-06T23:56:16Z

@swift-ci Please smoke test

Added tests for pointer types and changed the shouldInclude check to be less compact and prepare to accept @cdecl enums.

xymus · 2025-05-07T16:38:32Z

@swift-ci Please smoke test

xymus · 2025-05-08T18:33:19Z

@swift-ci Please smoke test

xymus · 2025-05-09T16:58:03Z

@swift-ci Please smoke test Windows

xymus · 2025-05-13T16:21:25Z

Merging to unblock the follow up PR adding support for #include and types imported from C.

This reverts commit 9c7f0c7.

xymus added 2 commits April 18, 2025 11:43

PrintAsObjc Test: Test compatibility headers for C

d5b3132

PrintAsClang: Intro and use getKnownType for C compatibility

e65a1fb

xymus requested review from zoecarver, egorzhdan, Xazax-hun, j-hui, fahadnayyar and susmonteiro as code owners April 18, 2025 19:01

xymus requested a review from beccadax April 18, 2025 19:01

xymus force-pushed the cdecl-print branch from 25f0279 to b739614 Compare April 18, 2025 20:11

beccadax requested changes Apr 18, 2025

View reviewed changes

xymus added 4 commits April 18, 2025 15:23

PrintAsObjC Test: Don't use -fobjc-arc when targeting the C language

bcbdfee

PrintAsObjC Test: Extract what needs Objective-C out of cdecl-official

a55b190

PrintAsClang: Ignore nullability attributes for C clients

20e1ac6

In C mode we still print nullability attributes. Don't let clang warn on them and ignore the attribute if the C compiler doesn't know them.

xymus force-pushed the cdecl-print branch from b739614 to 20e1ac6 Compare April 18, 2025 22:30

Xazax-hun reviewed Apr 22, 2025

View reviewed changes

PrintAsObjC Test: Ignore poison directories warnings for C

1b0a3d2

xymus added 3 commits May 6, 2025 16:18

PrintAsClang: Apply review comments to check on should print of @cdecl

3623c1d

PrintAsObjC Test: Don't use blocks in the @cdecl test

5e7887d

PrintAsObjC Test: Don't use modules for C compatibility header tests

0f98021

xymus force-pushed the cdecl-print branch from 4d9825b to 5ed3dc4 Compare May 6, 2025 23:53

PrintAsObjC Test: Test pointer types with @cdecl

f6dd432

xymus force-pushed the cdecl-print branch from 5ed3dc4 to f6dd432 Compare May 8, 2025 18:32

xymus merged commit 9c7f0c7 into swiftlang:main May 13, 2025
3 checks passed

xymus mentioned this pull request May 13, 2025

PrintAsClang: Print #include to import headers used from @cdecl functions #81481

Merged

hamishknight added a commit to hamishknight/swift that referenced this pull request May 22, 2025

Revert "Merge pull request swiftlang#80917 from xymus/cdecl-print"

f8972a4

This reverts commit 9c7f0c7.

PrintAsClang: Print a C block in the compatibility header for @cdecl functions #80917

PrintAsClang: Print a C block in the compatibility header for @cdecl functions #80917

Uh oh!

Conversation

xymus commented Apr 18, 2025

Uh oh!

xymus commented Apr 18, 2025

Uh oh!

beccadax commented Apr 18, 2025

Uh oh!

xymus commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xymus commented Apr 18, 2025

Uh oh!

beccadax left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beccadax Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beccadax Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xymus commented Apr 18, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xymus May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xymus commented Apr 30, 2025

Uh oh!

xymus commented Apr 30, 2025

Uh oh!

xymus commented May 5, 2025

Uh oh!

xymus commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xymus commented May 7, 2025

Uh oh!

xymus commented May 8, 2025

Uh oh!

xymus commented May 9, 2025

Uh oh!

xymus commented May 13, 2025

Uh oh!

Uh oh!

Uh oh!

PrintAsClang: Print a C block in the compatibility header for `@cdecl` functions #80917

PrintAsClang: Print a C block in the compatibility header for `@cdecl` functions #80917

xymus commented Apr 18, 2025 •

edited

Loading

beccadax Apr 18, 2025 •

edited

Loading

beccadax Apr 18, 2025 •

edited

Loading

xymus May 5, 2025 •

edited

Loading

xymus commented May 6, 2025 •

edited

Loading