Skip to content
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

[DNM/SILGen] Default argument generator visibility. #33452

Closed
wants to merge 1 commit into from

Conversation

johnno1962
Copy link
Contributor

@johnno1962 johnno1962 commented Aug 13, 2020

Hi Apple,

Thought I’d raise this PR as a possible change to make default argument generators non-hidden which would help me (in the long term) resolve an issue I’m seeing with “code Injection” for Swift. If a user tries to inject code which contains a call to a function using a default argument, InjectionIII fails as is shown in this issue. This is because while the function is public and accessible the default argument generators are "hidden" and the symbol is not available for dynamic linking which seems inconsistent.

Is there any chance it would be possible these functions be emitted public then this issue would not arise. @slavapestov, I see you made this commit for which I’m sure there are ABI “reasons” but could this be revisited specifically for default argument generators? At present I’m having to suggest users work around by running a program that patches object files to publish these symbols.

Cheers,

@johnno1962 johnno1962 changed the title Default argument generator visibility. [SILGen] Default argument generator visibility. Aug 13, 2020
@johnno1962 johnno1962 changed the title [SILGen] Default argument generator visibility. [DNM/SILGen] Default argument generator visibility. Aug 13, 2020
@shahmishal
Copy link
Member

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

  • How to change the base branch: (Link)
  • More detail about the branch update: (Link)

@johnno1962 johnno1962 changed the base branch from master to main October 1, 2020 20:57
@jckarter
Copy link
Member

The PublicNonABI is intentional. Default argument generators are not part of the ABI, they have purely compile-time effect, and a new SDK is allowed to change the default, or add defaults to existing arguments, without affecting ABI.

@jckarter jckarter closed this Apr 30, 2021
@johnno1962
Copy link
Contributor Author

Thanks for the response Joe. Would you entertain a patch that enabled this option with a command line switch or would that be too "niche"?

@jckarter
Copy link
Member

jckarter commented May 3, 2021

What are you trying to do?

@johnno1962
Copy link
Contributor Author

johnno1962 commented May 3, 2021

I have an open Source project https://github.com/johnno1962/InjectionIII which brings "Hot Reloading" to Swift apps by recompiling an individual source file into a .dylib, loading it and interposing new implementations on top of the old. One problem that crops up is that if you "inject" a file that uses a default argument, as their symbol has "hidden" visibility they are not visible to the dynamic linker and the load of the .dylib fails. I have a work-around of sorts but the real solution would be if the Swift compiler had an option such as "-export-default-arguments" people could use which changed the visibility as in this patch.

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.

None yet

3 participants