-
-
Notifications
You must be signed in to change notification settings - Fork 249
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
Unbox types properly from arrays #1476
Merged
workingjubilee
merged 68 commits into
pgcentralfoundation:develop
from
workingjubilee:hrtb-fns-and-unboxing-arrays
Jan 17, 2024
Merged
Changes from 58 commits
Commits
Show all changes
68 commits
Select commit
Hold shift + click to select a range
2141edd
Lift impl of FunctionMetadata to F: Fn(A) -> R
workingjubilee 44eaa4a
Allow fn pointers to need hrtb
workingjubilee 55c4867
preserve some code
workingjubilee 22de98e
Stop rewriting types to have zero spaces
workingjubilee 3f781b6
Anonymize lts in used types
workingjubilee 497cb5f
Use the named lifetime for composite_type!
workingjubilee 0d6fd48
Autofixes
workingjubilee ca2e321
rustfmt
workingjubilee 00168cd
Remove staticize_lifetimes
workingjubilee ef2c081
Remove broken staticize_lifetimes code
workingjubilee f4ee20f
Remove inline staticization in used_type.rs
workingjubilee 7de394a
Simplify FunctionMetadata impls for fn() -> R
workingjubilee e153bda
Allow using FnMut
workingjubilee 60e3d11
Commit the TypeId crime
workingjubilee 4cf2d41
well apparently that works
workingjubilee 641ab92
Erase unused code and fmt
workingjubilee 4e28358
Unify split_for_impl since no staticization
workingjubilee a0184f4
Comment on reasons
workingjubilee e8ead32
Add `Datum<'dat>`
workingjubilee 1704cd0
Add UnboxDatum
workingjubilee b244355
Add array leakage test
workingjubilee 7cf8fdc
Rampage over Array
workingjubilee 6106a14
Make arrays work with lifetimes
workingjubilee 5c2277c
Revert "Make arrays work with lifetimes"
workingjubilee 73930fb
Sorta-mostly fix arrays kinda
workingjubilee c8bf1ea
Make another bad example into a ui test
workingjubilee 2a574e9
Add unboxing for VariadicArray
workingjubilee d1e9ec3
Disable unsoundness tests
workingjubilee 31c41a0
Fix lifetime on json test
workingjubilee 99ca6d8
Make arbitrary PostgresEnums unboxable
workingjubilee a315e0f
Adding bounds to heap_tuple unboxing
workingjubilee c9e11da
Add notes on work-to-do
workingjubilee 4be285e
Docs and inlining hints for UnboxDatum
workingjubilee f13940f
Document more
workingjubilee 67062cc
Comment out the broken tests
workingjubilee b2cf030
Try to fix array iteration
workingjubilee aa2274d
Futz with lifetimes
workingjubilee e4118f8
Remove spare file
workingjubilee 6727d4c
Revert "Remove spare file"
workingjubilee 70c7d5d
Revert "Futz with lifetimes"
workingjubilee a6e1c42
Revert "Try to fix array iteration"
workingjubilee 9021dcf
Try to fix htup lifetimes
workingjubilee f496c76
Add unboxing for PostgresTypes
workingjubilee 44d431f
Prevent leaking lifetimes from HeapTuple
workingjubilee 33f8011
Fix postgresenum impl
workingjubilee a46dc31
Mark currently-unusable tests
workingjubilee 3707a21
Get over the line with some failing tests
workingjubilee 5bbc504
Infest arrays with pedantically correct lifetimes
workingjubilee 4c311b8
Fix last doctest
workingjubilee 54d3954
Erase redundant lifetime
workingjubilee 39f273f
fmt
workingjubilee af60efa
More lifetime pedantry
workingjubilee 816d464
More cleanup and strictly static
workingjubilee f0db83e
Fix test more properly
workingjubilee acaede9
Fix doc comment
workingjubilee 8270065
Minor cleanup
workingjubilee c71ec91
Fix most LT testing in roundtrip tests
workingjubilee 1740ad5
Add some TODOs
workingjubilee 1941050
Add todo tests as ui tests
workingjubilee eaee180
Mv compile-fail tests to a dir
workingjubilee 9d50280
Add serde test to todo tests
workingjubilee 91112f4
Add roundtrip tests to todo
workingjubilee 159a3a7
Fixup todo tests
workingjubilee ace035c
Add newlines because GH's EOF symbol annoys me
workingjubilee 9131c12
More test fixup
workingjubilee d6796d6
More fixes to test todos
workingjubilee a7b910d
Rename 'dat to 'src
workingjubilee 3b6cf39
fmt
workingjubilee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,76 +7,6 @@ | |
//LICENSE All rights reserved. | ||
//LICENSE | ||
//LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file. | ||
use crate::NameMacro; | ||
use proc_macro2::TokenStream; | ||
|
||
pub fn staticize_lifetimes_in_type_path(value: syn::TypePath) -> syn::TypePath { | ||
let mut ty = syn::Type::Path(value); | ||
staticize_lifetimes(&mut ty); | ||
match ty { | ||
syn::Type::Path(type_path) => type_path, | ||
|
||
// shouldn't happen | ||
_ => panic!("not a TypePath"), | ||
} | ||
} | ||
|
||
pub fn staticize_lifetimes(value: &mut syn::Type) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. goodby, |
||
match value { | ||
syn::Type::Path(syn::TypePath { path: syn::Path { segments, .. }, .. }) => segments | ||
.iter_mut() | ||
.filter_map(|segment| match &mut segment.arguments { | ||
syn::PathArguments::AngleBracketed(bracketed) => Some(bracketed), | ||
_ => None, | ||
}) | ||
.flat_map(|bracketed| &mut bracketed.args) | ||
.for_each(|arg| match arg { | ||
// rename lifetimes to the static lifetime so the TypeIds match. | ||
syn::GenericArgument::Lifetime(lifetime) => { | ||
lifetime.ident = syn::Ident::new("static", lifetime.ident.span()); | ||
} | ||
// recurse | ||
syn::GenericArgument::Type(ty) => staticize_lifetimes(ty), | ||
syn::GenericArgument::Binding(binding) => staticize_lifetimes(&mut binding.ty), | ||
syn::GenericArgument::Constraint(constraint) => { | ||
constraint.bounds.iter_mut().for_each(|bound| { | ||
if let syn::TypeParamBound::Lifetime(lifetime) = bound { | ||
lifetime.ident = syn::Ident::new("static", lifetime.ident.span()) | ||
} | ||
}) | ||
} | ||
// nothing to do otherwise | ||
_ => {} | ||
}), | ||
|
||
syn::Type::Reference(type_ref) => { | ||
if let Some(lifetime) = &mut type_ref.lifetime { | ||
lifetime.ident = syn::Ident::new("static", lifetime.ident.span()); | ||
} else { | ||
type_ref.lifetime = Some(syn::parse_quote!('static)); | ||
} | ||
} | ||
|
||
syn::Type::Tuple(type_tuple) => type_tuple.elems.iter_mut().for_each(staticize_lifetimes), | ||
|
||
syn::Type::Macro(syn::TypeMacro { mac }) | ||
if mac.path.segments.last().is_some_and(|seg| seg.ident == "name") => | ||
{ | ||
// We don't particularly care what the identifier is, so we parse a | ||
// raw TokenStream. Specifically, it's okay for the identifier String, | ||
// which we end up using as a Postgres column name, to be nearly any | ||
// string, which can include Rust reserved words such as "type" or "match" | ||
let Ok(out) = mac.parse_body::<NameMacro>() else { return }; | ||
let Ok(ident) = syn::parse_str::<TokenStream>(&out.ident) else { return }; | ||
let mut ty = out.used_ty.resolved_ty; | ||
|
||
// rewrite the name!() macro's type so that it has a static lifetime, if any | ||
staticize_lifetimes(&mut ty); | ||
*mac = syn::parse_quote! {::pgrx::name!(#ident, #ty)}; | ||
} | ||
_ => {} | ||
} | ||
} | ||
|
||
pub fn anonymize_lifetimes_in_type_path(value: syn::TypePath) -> syn::TypePath { | ||
let mut ty = syn::Type::Path(value); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question... why this change?
Could it not be
Vec<Option<&'a str>>
?This was the only example you touched, so either that means this example is the only one that accepts a
&str
and that pattern is no longer valid, or you changed this for some other reason?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this changes FromDatum in the impls in pgrx/src/datum/array.rs to be in terms of
T: UnboxDatum
with certain bounds, this PR currently breaksFromDatum
specifically forVec<Option<T>>
where there is any doubt as to whetherT
implementsUnboxDatum
. Essentially, the trait resolver can't figure it out at that depth. This can be solved by involving additional traits which work differently and either allowVec<Option<T>>
to not depend directly onArray<'_, T>
for its unboxing or to movepg_getarg
(or its replacement) offT: FromDatum
.