Update 2 syn #1557

workingjubilee · 2024-02-13T18:14:07Z

We have not been receiving updates to syn for a while, because we are on syn 1 still. Update to syn 2 and lift it into the workspace, which requires revamping how several proc_macro_attributes are parsed. In the process, sprinkle pgrx-sql-entity-graph with paranoid amounts of track_caller, expects, and compound errors on the unhappy path, to try to make the code start producing useful error spans.

Then also drop unused features from some of our dependencies to completely erase syn 1 from the dependency tree. We don't meddle with any compressible sections, because we just look at exported symbols, not debuginfo, so I believe this is inconsequential. It's acceptable to revert that part of this change, however, if it doesn't actually work out.

Removes 4 crates from the dependency tree

The syn changes include:

syn::LifetimeDef is now LifetimeParam
syn::ImplItem::Method is now syn::ImplItem::Fn
syn::ImplItemMethod is now syn::ImplItemFn
Type ascription && syn::Expr::Type no longer exist
syn::MetaNameValue replaces its lit field with value, as it accepts non-literals
syn::GenericArgument::Binding is now GenericArgument::AssocType
syn::Attribute hid its path field as .path()
syn::Attribute hid its token field, so there is only ToTokens
syn::Attribute changed how it structures access to its args. We now parse inner args of a syn::Attribute for our proc-macro attributes like #[pg_operator], without considering parentheses.

This does not fix everything because some things were restructured.

We were expecting the parentheses, but now we do not get those. Instead, drop parens from our structs and parse directly.

This required several hours of hunting and is one of the consequences of the attribute layering change.

Considering only the inner args of the attribute is necessary in order to get the correct names so that we can emit correct SQL. Ideally we would have much more resilient parsing that could extract data out of a broader range of token streams passed in. However, that is not the case currently, so don't.

workingjubilee · 2024-02-13T18:30:36Z

cc @NotGyro

eeeebbbbrrrr

This looks largely mechanical, which is good. I’m sure it was a process tho!

NotGyro

Other than the small error message issue and the comment I think this is good. Reading through Syn's changelog, there shouldn't be any silent breaking changes (i.e. if there's a breaking change that we haven't taken into account it won't let us compile) and this builds and passes tests on my machine so it looks like it should be all set. The most common edit I'm seeing here is the parse_terminated() change and that all looks correct.

As an aside, I'm glad Binding got split into AssocType and AssocConst.

pgrx-sql-entity-graph/src/extern_args.rs

pgrx-sql-entity-graph/src/pg_extern/mod.rs

workingjubilee

Eliminating 4 dependencies is nice, but we shouldn't pass up easy points on which to make the nightmarish code here more maintainable.

pgrx-macros/src/lib.rs

pgrx-sql-entity-graph/src/pg_extern/mod.rs

pgrx-macros/src/lib.rs

workingjubilee · 2024-02-13T22:43:27Z

pgrx-sql-entity-graph/src/fmt.rs

+macro_rules! lazy_err {
+    ($span_expr:expr, $lit:literal $(, $tokens:tt),*) => {
+        {
+            let spanning = $span_expr;


This is so that the expr can resolve instead of being captured into the closure. This make the name lazy_err! slightly confusing, perhaps, since this part is eagerly-evaluated, but a lot of exprs die instead.

Fixed by adding docs.

pgrx-sql-entity-graph/src/fmt.rs

cargo-pgrx/Cargo.toml

NotGyro

Looks good to me after the changes!

Per Syn's docs, the replacement of parse2() with parse_args() should be fine, since:

This is similar to pulling out the TokenStream from Meta::List and doing syn::parse2::(meta_list.tokens), except that using parse_args the error message has a more useful span when tokens is empty.

(quote from here )

@usamoi

Welcome to pgrx 0.12.0-alpha.1! Say the magic words with me! ```shell cargo install cargo-pgrx --locked --version 0.12.0-alpha.1 ``` # Breaking Changes ## No more dlopen! Perhaps the most exciting change this round is @usamoi's contribution in #1468 which means that we no longer perform a `dlopen` in order to generate the schema. The cost, such as it is, is that your pgrx extensions now require a `src/bin/pgrx_embed.rs`, which will be used to generate the schema. This has much less cross-platform issues and will enable supporting things like `cargo binstall` down the line. It may be a bit touchy on first-time setup for transitioning older repos. If necessary, you may have to directly add a `src/bin/pgrx_embed.rs` and add the following code (which should be the only code in the file, though you can add comments if you like?): ```rust ::pgrx::pgrx_embed!(); ``` Your Cargo.toml will also want to update its crate-type key for the library: ```toml [lib] crate-type = ["cdylib", "lib"] ``` ## Library Code - pgrx-pg-sys will now use `ManuallyDropUnion` thanks to @NotGyro in #1547 - VARHDRSZ `const`s are no longer `fn`, thanks to @workingjubilee in #1584 - We no longer have `Interval::is_finite` since #1594 - We translate more `*_tree_walker` functions to the same signature their `*_impl` version in Postgres 16 has: #1596 - Thanks to @eeeebbbbrrrr in #1591 we no longer have the `pg_sql_graph_magic!()` macro, which should help with more things in the future! # What's New We have quite a lot of useful additions to our API: - `SpiClient::prepare_mut` was added thanks to @XeniaLu in #1275 - @usamoi also contributed bindings subscripting code in #1562 - For `#[pg_test]`, you have been able to use `#[should_panic(expected = "string")]` to anticipate a panic that contains that string in that test. For various reasons, `#[pg_test(error = "string")]` is much the same. Now, you can also use `#[pg_test(expected = "string")]`, in the hopes that is easier to stumble across, as of #1570 ## `Result<composite_type!("..."), E>` support - In #1560 @NotGyro contributed support for using `Result<composite_type!("Name"), E>`, as a case that had not been handled before. ## Significantly expanded docs Thanks to @rjuju, @NotGyro, and @workingjubilee, we now have significantly expanded docs for cargo-pgrx and pgrx in general. Some of these are in the API docs on https://docs.rs or the READMEs, but there's also a guide, now! It's not currently published, but is available as an [mdbook](https://github.com/rust-lang/mdBook) in the repo. Some diagnostic information that is also arguably documentation, like comments and the suggestion to `cargo install`, have also been improved, thanks to @workingjubilee in - #1579 - #1573 ## `#[pg_cast]` An experimental macro for a `CREATE CAST` was contributed by @xwkuang5 in #1445! ## Legal Stuff Thanks to @the-kenny in #1490 and @workingjubilee in #1504, it was brought to our attention that some dependencies had unusual legal requirements. So we fixed this with CI! We now check our code included into pgrx-using binaries is MIT/Apache 2.0 licensed, as is common across crates.io, using `cargo deny`!. The build tools will have more flexible legal requirements (partly due to the use of Mozilla Public License code in rustls). # Internal Changes Many internal cleanups were done thanks to - @workingjubilee in too many PRs to count! - @thomcc found a needless condition in #1501 - @nyurik in too many PRs to count! In particular: - we now actually `pfree` our `Array`s we detoasted as-of #1571 - creating a `RawArray` is now low-overhead due to #1587 ## Soundness Fixes We had a number of soundness issues uncovered or have added more tests to catch them. - Bounds-checking debug assertions for array access by @NotGyro in #1514 - Fix unsound `&` and `&mut` in `fcinfo.rs` by @workingjubilee in #1595 ## Less Deps Part of the cleanup by @workingjubilee was reducing the number of deps we compile: * cargo-pgrx: reduce trivial dep usages in #1499 * Update 2 syn in #1557 Hopefully it will reduce compile time and disk usage! ## New Contributors * @the-kenny made their first contribution in #1490 * @xwkuang5 made their first contribution in #1445 * @rjuju made their first contribution in #1516 * @nyurik made their first contribution in #1533 * @NotGyro made their first contribution in #1514 * @XeniaLu made their first contribution in #1275 **Full Changelog**: v0.12.0-alpha.0...v0.12.0-alpha.1

workingjubilee added 17 commits February 12, 2024 17:11

Update to syn 2, applying renames

347e76c

This does not fix everything because some things were restructured.

MetaNameValues accept more than literals

1f0365c

Type ascription && Expr::Type no longer exists

b87a9e0

Attribute::path is an impl fn

32c03e1

Attribute hid its tokens field

8748dbb

GenericArgument::Binding is now ::AssocType

8fc3da8

Drop some unused code imports

e906f07

::parse_terminated requires a separator arg

607f27d

Attempt to lift proc-macro2, quote, syn to workspace

2961721

Add superstitious amounts of tracing to find syn err

cd9c59d

syn::Attribute yields paren-free args

8c942a2

We were expecting the parentheses, but now we do not get those. Instead, drop parens from our structs and parse directly.

Fix pg_aggregate + pgrx(..)

cfd6002

This required several hours of hunting and is one of the consequences of the attribute layering change.

Add some debugging info

0971844

Small fixup in example

8325337

Drop the unused syn fold feature

d85235d

Drop object default features to drop last syn 1

98f19db

eeeebbbbrrrr reviewed Feb 13, 2024

View reviewed changes

NotGyro suggested changes Feb 13, 2024

View reviewed changes

pgrx-sql-entity-graph/src/extern_args.rs Show resolved Hide resolved

pgrx-sql-entity-graph/src/pg_extern/mod.rs Outdated Show resolved Hide resolved

workingjubilee commented Feb 13, 2024

View reviewed changes

pgrx-macros/src/lib.rs Outdated Show resolved Hide resolved

pgrx-sql-entity-graph/src/pg_extern/mod.rs Outdated Show resolved Hide resolved

pgrx-sql-entity-graph/src/pg_extern/mod.rs Outdated Show resolved Hide resolved

pgrx-macros/src/lib.rs Outdated Show resolved Hide resolved

workingjubilee added 3 commits February 13, 2024 13:55

Use syn::Error::into_compile_error consistently

68c372f

Factor out error-handling

19939b7

rustfmt again

0bb9159

workingjubilee commented Feb 13, 2024

View reviewed changes

Document pgrx_sql_entity_graph::fmt::lazy_err

325b6c2

workingjubilee commented Feb 13, 2024

View reviewed changes

cargo-pgrx/Cargo.toml Outdated Show resolved Hide resolved

Delete random newline

f6495db

NotGyro approved these changes Feb 13, 2024

View reviewed changes

workingjubilee merged commit c165735 into pgcentralfoundation:develop Feb 14, 2024
8 checks passed

workingjubilee mentioned this pull request Mar 1, 2024

Release pgrx 0.12.0-alpha.1 #1598

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update 2 syn #1557

Update 2 syn #1557

workingjubilee commented Feb 13, 2024 •

edited

workingjubilee commented Feb 13, 2024

eeeebbbbrrrr left a comment

NotGyro left a comment

workingjubilee left a comment

workingjubilee Feb 13, 2024

workingjubilee Feb 13, 2024

NotGyro left a comment

Update 2 syn #1557

Update 2 syn #1557

Conversation

workingjubilee commented Feb 13, 2024 • edited

workingjubilee commented Feb 13, 2024

eeeebbbbrrrr left a comment

Choose a reason for hiding this comment

NotGyro left a comment

Choose a reason for hiding this comment

workingjubilee left a comment

Choose a reason for hiding this comment

workingjubilee Feb 13, 2024

Choose a reason for hiding this comment

workingjubilee Feb 13, 2024

Choose a reason for hiding this comment

NotGyro left a comment

Choose a reason for hiding this comment

workingjubilee commented Feb 13, 2024 •

edited