-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add -parse-as-library to Swift library targets #14298
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
base: master
Are you sure you want to change the base?
Conversation
fad110c
to
e9a67a2
Compare
de65bfe
to
b210239
Compare
b210239
to
f1c04f0
Compare
f1c04f0
to
46ad197
Compare
Would it be simpler to always add this flag when compiling library targets that have only one Swift source file? Is there ever a case where it would not work? |
That's implemented like that in #14267 (well, it adds it to all library targets but that doesn't really make a difference). Sometimes you want it to be there on executables as well however, if you have an explicit @main annotation on a type for example (and I also didn't want to force it to be there on libraries with no way to disable it), so this is the version with a toggle flag. One of the situations in which #14267 would not work is if you want to provide a main function in a Swift library that you then link into an executable which does not have a main. Though even there you could use the explicit @main annotation on a type in the library. Unconventional but I've seen it done in C++. If you think #14267 would be better to merge, that's fine with me (though I can't reopen it now and would have to recreate it... thanks GitHub). |
Having an empty string as a potential value is not great UX. It should always be a non-empty string that unambiguously specifies what the value does. So maybe the choices should be |
Hm, originally I had 'always', 'auto' and 'never' which I then changed to what it is now since 'always' doesn't actually always enable this. 'yes' would be a better name for it, yeah. I agree on the empty value being bad UX. However, here's a slight variation on your first suggestion that I think should make it do the right thing in almost all cases, without needing a kwarg: What do you think about adding the flag automatically to a target if the target is a library, it has only one file and the file is not called main.swift? I think that should get generally reasonable behavior for libraries, where only main.swift is treated as toplevel code if it's there and never files with other names, and the default behavior without the flag for executables. The only edge case would remain is executable targets with a single source file that want to use a @main annotation or link in main from a library. I think it's reasonable to just add it to swift_flags there, my main concern was being able to disable adding of the flag. |
fd9d29e
to
0190655
Compare
Can you add two test cases, one for a library that only has a main.swift file and one for a library that has main.swift and other files? Also please add documentation and release notes. |
As an aside it would be great to add per-language documentation (especially for Swift and Rust), so if you add such a section for Swift I will gladly review it. |
b35de1f
to
bf858f6
Compare
What documentation would you like to have other than the release notes? |
Not specific to this patch, anything that people using Swift with Meson instead of Xcode would need to know. That is, more of a tutorial/how to than a reference. This is unrelated to this PR. |
Ah, I see. That would be useful, I'll see if I can write something up when I have the energy :^) |
5182285
to
6c23435
Compare
6c23435
to
aff75b9
Compare
swiftc adds a main function consisting of top-level statements to objects generated by a swiftc invocation with a single input file, leading to duplicate symbols when linking the final executable (in case of a static library). This argument prevents this from happening, apply it to library targets automatically when the single file is not called main.swift (in order to preserve the behavior if wanted).