-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Remove _silgen_name from all Apple overlays except Dispatch and Foundation #5309
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
Remove _silgen_name from all Apple overlays except Dispatch and Foundation #5309
Conversation
This does require a dummy protocol for now; hopefully we can take it out later.
Loading a Clang module eagerly brings in overlays for anything it re-exports, but this is a problem for these new "shim header" modules, which generally import the underlying module for an overlay and are in turn imported by the overlay. That means that when we try to import an overlay, we'll end up with a circular reference before it's done loading all its dependencies. Break the cycle by not exporting anything from these modules, which are mostly just an implementation detail anyway.
These get imported by Swift, so "Objective-C" would be more appropriate. For now, though, just let it fall back to C mode.
@moiseev, @slavapestov, can you review these changes? @swift-ci Please test macOS platform @swift-ci Please smoke test Linux platform |
@swift-ci Please test macOS platform |
Build failed |
The |
Thanks for trying to clean this up! Maybe we should also hide |
That's my ultimate goal (and I want the flag to be |
Cool. LLDB and Playgrounds shouldn't be too much of a problem; the former already has its own special frontend mode, and the Playground logger directly manipulates ASTs, so doesn't need to be subjected to parser-level restrictions. |
Max talked to me in person a few days ago, and Slava left behind an emoji, so I'm going to assume this is okay to merge and possibly undergo refinement later. Retesting just to be safe. @swift-ci Please test macOS |
Steps towards getting rid of
_silgen_name
altogether, which is generally unsafe and not something we're interested in supporting in its current form. I left out Dispatch and Foundation for now because those have code owners active in the Swift project. The "platform" overlay (Darwin/Glibc) has also been exempted for now because I didn't want to poke at any low-level import dependencies.There are a few downsides to all this: these headers get installed along with the shims even though they're only used to build the overlays, and only on Apple platforms. I or someone else should go back later and
Overall, though, I think this is a safety and simplicity win.