Skip to content

[5.9] Diagnose attempts to reabstract variadic function types in unimplementable ways #67385

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

Conversation

rjmccall
Copy link
Contributor

@rjmccall rjmccall commented Jul 19, 2023

5.9 version of #67368. Fixes rdar://112506013.

Description:

Swift’s generics system’s ability to (1) directly run generic code while (2) making calls using the best-possible calling convention for the declared type of a function relies on the ability to “reabstract” function values: wrap them in functions that have the same logical type but use a different calling convention (CC). Some cases in variadic generics require a kind of function reabstraction that turns out to be unimplementable in Swift’s ABI without heroic effort. We need to detect code that relies on this reabstraction and emit an error. If we fail to do this, we could get stuck supporting code that works “if you hold it right” but is unsound in general.

Specifically, variadic generics adds the ability to abstract over the arity of a function, e.g. to generalize a function type like (Int, Float) -> Bool not just as (T, U) -> V but as (repeat each T) -> V. The natural CC for (Int, Float) -> Bool takes the parameter values directly in two registers. The natural CC for (T, U) -> V takes the parameters as two pointers to the values. The natural CC for (repeat each T) -> V takes the parameters as a pointer to an array of pointers to the values.

Suppose that we assign a function of type (Int, Float) -> Bool into a variable of type Any. What CC do we expect that stored function to use? If it’s the natural convention for the unabstracted type, then we won’t be able to extract it successfully in a generic context. For example, suppose we cast it to (Int, T) -> Bool, where T happens to dynamically be Float. We expect this cast to work, because the type we’re asking for is right. But our generic context wouldn’t actually be able to call the function with an unabstracted CC because it doesn’t know statically that T is Float and so doesn’t know how to pass that argument. We make this work by picking a “most general” abstraction and expecting these “fully abstracted” values to use the CC for that pattern. For (Int, Float) -> Bool, the ABI says that this pattern is (T, U) -> V, and so that’s the CC we expect the function value in the Any to use. If our generic context specifically needs a function with the conventions of (Int, T) -> Bool, it can reabstract the function it gets from the Any to use that CC: it wraps the function in a function that takes the Int directly in a register, stores it to memory, and then passes the address of that memory to the original function.

This algorithm breaks down with variadic generics because our ABI didn’t anticipate abstraction over arity. In formal terms, the problem is that there are function types like (repeat each T) -> Bool that can be substituted to produce (Int, Float) -> Bool but which can’t be produced with a substitution from the “most general” function type of (T, U) -> V. In practical terms, if we try to reabstract a “most general” function value to a variadically-generic function type, we statically know that the function is supposed to take some number of separate pointer arguments but we don’t know how many, and so we can’t emit a normal function that would map between the two conventions without doing something like dynamically interpreting the calling convention, which would require platform-specific assembly code. We can consider doing that in the future, but in the meantime, we have a problem.

Worse, the code will actually work at runtime as long as we only put functions in and out with the same variadically-generic function type. But there’s no way to statically verify that use-pattern; it’s completely dynamically unsound. So we need to lock this down before we have a body of code relying on that unsound behavior.

Scope: Diagnostic to prevent compilation of certain kinds of reabstraction thunk.
Risk: Low. Code is written in a way that should avoid impacting other cases. Most likely failure mode is false-negative, i.e. we fail to diagnose these unimplementable cases in some situation I haven't accounted for. With this patch in place, though, we should be able to defend future diagnostics in such situations as bug fixes and accept the source break.
Reviewed by: Slava Pestov
Testing: CI, new test

@rjmccall rjmccall requested a review from a team as a code owner July 19, 2023 01:34
@rjmccall
Copy link
Contributor Author

@swift-ci Please test

@rjmccall rjmccall merged commit 298f2df into swiftlang:release/5.9 Jul 19, 2023
@rjmccall rjmccall deleted the diagnose-impossible-variadics-5.9 branch July 19, 2023 21:47
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.

3 participants