Skip to content

install a list of symbols generated by the PrintAsClang header #59072

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 2 commits into from
Dec 9, 2022

Conversation

QuietMisdreavus
Copy link
Contributor

Resolves rdar://93504690

This PR adds a new file that is installed with the compiler: compatibility_symbols. This is a list of the macros defined in the compatibility header generated by PrintAsClang when exposing Swift symbols to Objective-C. The idea behind creating this file was that tools that parsed C-based headers for symbols (e.g. the new clang-extractapi) could use this file to filter out symbols that were not part of the user's own code.

@QuietMisdreavus QuietMisdreavus requested a review from gottesmm May 25, 2022 16:47
@QuietMisdreavus
Copy link
Contributor Author

@gottesmm Is it possible to reference this file from a lit test? I would love to be able to verify that the generated compatibility header actually matches the list of symbols in the installed file, but i'm not sure the best way to approach it.

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please smoke test

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please build toolchain macOS

@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/compat-syms branch from 57f9c17 to f2c6a9b Compare June 9, 2022 17:14
@QuietMisdreavus
Copy link
Contributor Author

@gottesmm I've overhauled the PR to generate the PrintAsClang macros via a definition file. I then took this file and used it in a new host tool to dynamically generate the list of compatibility symbols instead of making it a static file. Can you take another look?

@QuietMisdreavus QuietMisdreavus marked this pull request as ready for review June 9, 2022 17:17
@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

2 similar comments
@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/compat-syms branch from bf0da08 to e17c615 Compare June 10, 2022 16:06
@QuietMisdreavus
Copy link
Contributor Author

Rebased to handle merge conflict.

@swift-ci Please test

@@ -0,0 +1,38 @@
IBSegueAction
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: These are not symbols in the usual sense, since they aren’t linkable. “Identifier”? “Macro?”

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're "symbols" in the sense that they would be emitted in a symbol graph and appear in documentation, though. "Symbols for documentation" has a looser definition than "symbols for linking".

@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/compat-syms branch from e17c615 to 915ade4 Compare June 23, 2022 21:05
@QuietMisdreavus
Copy link
Contributor Author

Rebased to handle merge conflict.

@swift-ci Please test

@QuietMisdreavus QuietMisdreavus requested a review from hyp July 11, 2022 15:20
@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/compat-syms branch from 915ade4 to 0ea00fd Compare September 16, 2022 19:22
@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

#define CLANG_MACRO_CXX_BODY(NAME, BODY) \
CLANG_MACRO_DEFINED(NAME)

#include "swift/PrintAsClang/ClangMacros.def"
Copy link
Contributor

Choose a reason for hiding this comment

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

We also need to handle mapped SIMD types, maybe something along the lines of:

#define MAP_SIMD_TYPE(C_TYPE, SCALAR_TYPE, _)    \
    CLANG_MACRO_DEFINED("swift_" #C_TYPE "2")    \
    CLANG_MACRO_DEFINED("swift_" #C_TYPE "3")    \
    CLANG_MACRO_DEFINED("swift_" #C_TYPE "4")    

@QuietMisdreavus
Copy link
Contributor Author

Rebased to the latest main, squashed the commits, and added a new one that addresses @daniel-grumberg's comment.

@swift-ci Please test

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please build toolchain

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus QuietMisdreavus merged commit 8d548ef into main Dec 9, 2022
@QuietMisdreavus QuietMisdreavus deleted the QuietMisdreavus/compat-syms branch December 9, 2022 23:38

CLANG_MACRO_DEFINED("SWIFT_TYPEDEFS")

#define MAP_SIMD_TYPE(C_TYPE, SCALAR_TYPE, _) \
Copy link
Contributor

Choose a reason for hiding this comment

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

These aren't macros but rather typedefs. Would it make more sense for this logic to go into the actual executable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like the purpose of this file is mainly to accumulate the symbols that PrintAsClang adds to the compatibility header, whether they're macros or not. The infrastructure treats them all as macros, but the end result is the same: We can use the resulting list in clang-extractapi to filter out the symbols that didn't actually come from user code.

quinntaylor pushed a commit to quinntaylor/swift that referenced this pull request Dec 12, 2022
beccadax added a commit to beccadax/swift that referenced this pull request Feb 1, 2023
swiftlang#59072 accidentally changed the SWIFT_CLASS_NAMED macro to use `__attribute` when it previously used `__attribute__` (note the trailing underscores). While both keywords have the same semantics in clang, they are technically different tokens, so clang refuses to merge macro definitions that use one instead of the other; instead it would diagnose an ambiguity when a generated header from a new compiler imported a generated header from an old compiler. Change back to the old token to avoid this problem.

Fixes rdar://104252758.
QuietMisdreavus pushed a commit that referenced this pull request Mar 1, 2023
#59072 accidentally changed the SWIFT_CLASS_NAMED macro to use `__attribute` when it previously used `__attribute__` (note the trailing underscores). While both keywords have the same semantics in clang, they are technically different tokens, so clang refuses to merge macro definitions that use one instead of the other; instead it would diagnose an ambiguity when a generated header from a new compiler imported a generated header from an old compiler. Change back to the old token to avoid this problem.

Fixes rdar://104252758.
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.

5 participants