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

Manually staticize lifetimes in pgrx tests #1461

Conversation

workingjubilee
Copy link
Member

These all are actually lifetimes bound by their invocation site, which means that they return any lifetime which is named, instead of a lifetime that is bound by the input types in a logical way. This means that these lifetimes might as well be 'static, for all it would (not) matter.

An important detail is that while this pattern is normally acceptable and useful in Rust code, in pgrx it should be rejected for now because the lifetimes on the actually-called-by-Postgres function are defined, essentially, by a function pointer. These lifetimes cannot be coherently described to a function pointer. Thus, they form a sort of impedance mismatch: when you call this function from Rust code, you get a constraint that doesn't actually exist in the code that will run in Postgres. The difference is confusing, allowing you to write code relying on the compiler's lifetime checks and even fail to call it in certain ways, seemingly satisfying the soundness requirements. Then suddenly the assumption those compiler checks are founded on evaporates in the actual runtime execution, leading to undefined behavior.

I hope to fix a number of these problems relatively soon, but first thing's first: The removal of illusions. If this code is unsound now, then in truth it already was.

This effectively serves as a followup to #1457 and it is part of what enables solving #1383 by not requiring the pervasive use of that hack.

These all are actually lifetimes bound by their invocation site,
which means that they return any lifetime which is named, instead
of a lifetime that is bound by the input types in a logical way.
This means that these lifetimes might as well be 'static, for all
it would (not) matter.
@workingjubilee
Copy link
Member Author

For an example of a legitimate use of this pattern: An 'invariant lifetime would not unify with 'static. The problem is that you can't describe the lifetime of Invariant<'lt> to a function pointer, and the function pointer is what is ultimately truly called.

@workingjubilee workingjubilee merged commit 5aa76f0 into pgcentralfoundation:develop Jan 4, 2024
8 checks passed
@workingjubilee workingjubilee deleted the staticize-unconstrained-return-lifetimes branch January 4, 2024 18:45
workingjubilee added a commit that referenced this pull request Jan 5, 2024
The `pgrx::composite_type!` macro currently assigns the `'static`
lifetime if you do not give one. This is a decision of dubious
soundness. Instead, demand a lifetime, or assign the anonymous lifetime.
Both cases match the behavior of attempting to elide lifetimes in your
usual Rust code, though with slightly worse error messages in some
cases. This may require people to write `'static` in more places (and
that is what the typical error suggests), but it allows noticing and
fixing problems that currently are not visible, easy to understand, or
easy to check.

This is the latest in this series:
- #1383
- #1457
- #1461
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.

1 participant