Skip to content

Commit

Permalink
Begin synthesizing spans (#1746)
Browse files Browse the repository at this point in the history
I consulted other rust-lang devs and they agree proc-macros should use
what I refer to as "synthetic" spans. These spans have their root
context derived from `Span::mixed_site()` or `Span::call_site()` but
have been given a location in the code somewhere.

- Fixes remaining `used_underscore_binding` warnings.
- Closes #1667
  • Loading branch information
workingjubilee authored Jul 3, 2024
1 parent 8057092 commit 2c1d388
Show file tree
Hide file tree
Showing 16 changed files with 107 additions and 21 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ jobs:

# We can't do this with other lints because we need $PGRX_HOME
- name: Clippy -Awarnings
run: cargo clippy -p pgrx --features pg$PG_VER -- -Awarnings
run: cargo clippy --tests --features pg$PG_VER --no-default-features -- -Awarnings

# This also requires $PGRX_HOME
- name: Check doc-links
Expand Down
16 changes: 14 additions & 2 deletions pgrx-macros/src/rewriter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
//LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file.
extern crate proc_macro;

use quote::{format_ident, quote, quote_spanned};
use quote::{quote, quote_spanned};
use std::mem;
use std::ops::Deref;
use std::str::FromStr;
Expand All @@ -20,6 +20,14 @@ use syn::{
Visibility,
};

macro_rules! format_ident {
($s:literal, $e:expr) => {{
let mut synthetic = $e.clone();
synthetic.set_span(proc_macro2::Span::call_site().located_at($e.span()));
quote::format_ident!($s, synthetic)
}};
}

pub fn extern_block(block: ItemForeignMod) -> proc_macro2::TokenStream {
let mut stream = proc_macro2::TokenStream::new();

Expand Down Expand Up @@ -60,6 +68,7 @@ pub fn item_fn_without_rewrite(mut func: ItemFn) -> syn::Result<proc_macro2::Tok

let arg_list = build_arg_list(&sig, false)?;
let func_name = func.sig.ident.clone();
let func_name = format_ident!("{}", func_name);

let prolog = if input_func_name == "__pgrx_private_shmem_hook"
|| input_func_name == "__pgrx_private_shmem_request_hook"
Expand Down Expand Up @@ -90,7 +99,9 @@ pub fn item_fn_without_rewrite(mut func: ItemFn) -> syn::Result<proc_macro2::Tok
quote! { #func_name::<#ty>(#arg_list) }
};

Ok(quote_spanned! {func.span()=>
let synthetic = proc_macro2::Span::mixed_site().located_at(func.span());

Ok(quote_spanned! {synthetic=>
#prolog
#(#attrs)*
#vis #sig {
Expand Down Expand Up @@ -149,6 +160,7 @@ fn build_arg_list(sig: &Signature, suffix_arg_name: bool) -> syn::Result<proc_ma
let ident = format_ident!("{}_", ident.ident);
arg_list.extend(quote! { #ident, });
} else {
let ident = format_ident!("{}", ident.ident);
arg_list.extend(quote! { #ident, });
}
} else {
Expand Down
5 changes: 4 additions & 1 deletion pgrx-sql-entity-graph/src/finfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ pub fn finfo_v1_extern_c(
let original_name = &original.sig.ident;
let wrapper_symbol = format_ident!("{}_wrapper", original_name);

let tokens = quote_spanned! { original.sig.span() =>
let synthetic = proc_macro2::Span::mixed_site();
let synthetic = synthetic.located_at(original.sig.span());

let tokens = quote_spanned! { synthetic =>
#[no_mangle]
#[doc(hidden)]
pub unsafe extern "C" fn #wrapper_symbol(#fcinfo: ::pgrx::pg_sys::FunctionCallInfo) -> ::pgrx::pg_sys::Datum {
Expand Down
23 changes: 21 additions & 2 deletions pgrx-sql-entity-graph/src/pg_extern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,30 @@ use operator::{PgrxOperatorAttributeWithIdent, PgrxOperatorOpName};
use search_path::SearchPathList;

use proc_macro2::{Ident, Span, TokenStream as TokenStream2};
use quote::{format_ident, quote, quote_spanned};
use quote::quote;
use syn::parse::{Parse, ParseStream, Parser};
use syn::punctuated::Punctuated;
use syn::spanned::Spanned;
use syn::{Meta, Token};

macro_rules! quote_spanned {
($span:expr=> $($expansion:tt)*) => {
{
let synthetic = Span::mixed_site();
let synthetic = synthetic.located_at($span);
quote::quote_spanned! {synthetic=> $($expansion)* }
}
};
}

macro_rules! format_ident {
($s:literal, $e:expr) => {{
let mut synthetic = $e.clone();
synthetic.set_span(Span::call_site().located_at($e.span()));
quote::format_ident!($s, synthetic)
}};
}

/// A parsed `#[pg_extern]` item.
///
/// It should be used with [`syn::parse::Parse`] functions.
Expand Down Expand Up @@ -377,7 +395,8 @@ impl PgExtern {
let signature = &self.func.sig;
let func_name = &signature.ident;
// we do this odd dance so we can pass the same ident to macros that don't know each other
let fcinfo_ident = syn::Ident::new("fcinfo", signature.ident.span());
let synthetic_ident_span = Span::mixed_site().located_at(signature.ident.span());
let fcinfo_ident = syn::Ident::new("fcinfo", synthetic_ident_span);
let mut lifetimes = signature
.generics
.lifetimes()
Expand Down
5 changes: 3 additions & 2 deletions pgrx-sql-entity-graph/src/pg_trigger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
use crate::finfo::{finfo_v1_extern_c, finfo_v1_tokens};
use crate::{CodeEnrichment, ToSqlConfig};
use attribute::PgTriggerAttribute;
use proc_macro2::{Ident, TokenStream as TokenStream2};
use proc_macro2::{Ident, Span, TokenStream as TokenStream2};
use quote::{format_ident, quote};
use syn::{spanned::Spanned, ItemFn, Token};

Expand Down Expand Up @@ -70,7 +70,8 @@ impl PgTrigger {

pub fn wrapper_tokens(&self) -> Result<ItemFn, syn::Error> {
let function_ident = self.func.sig.ident.clone();
let fcinfo_ident = Ident::new("_fcinfo", function_ident.span());
let fcinfo_ident =
Ident::new("_fcinfo", Span::mixed_site().located_at(function_ident.span()));

let tokens = quote! {
fn _internal(fcinfo: ::pgrx::pg_sys::FunctionCallInfo) -> ::pgrx::pg_sys::Datum {
Expand Down
2 changes: 1 addition & 1 deletion pgrx-tests/src/tests/attributes_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ mod tests {
#[pg_test]
#[ignore = "This test should be ignored."]
fn test_for_ignore_attribute() {
assert_eq!(true, true);
assert_eq!(true, false);
}

#[pg_test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ error[E0521]: borrowed data escapes outside of function
| lifetime `'fcx` defined here
| argument requires that `'fcx` must outlive `'static`
|
= note: this error originates in the attribute macro `pg_aggregate` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the attribute macro `::pgrx::pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0521]: borrowed data escapes outside of function
--> tests/compile-fail/aggregate-functions-dont-run-forever.rs:47:1
Expand All @@ -22,7 +22,7 @@ error[E0521]: borrowed data escapes outside of function
| lifetime `'fcx` defined here
| argument requires that `'fcx` must outlive `'static`
|
= note: this error originates in the attribute macro `pg_aggregate` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the attribute macro `::pgrx::pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0521]: borrowed data escapes outside of function
--> tests/compile-fail/aggregate-functions-dont-run-forever.rs:70:1
Expand All @@ -35,7 +35,7 @@ error[E0521]: borrowed data escapes outside of function
| lifetime `'fcx` defined here
| argument requires that `'fcx` must outlive `'static`
|
= note: this error originates in the attribute macro `pg_aggregate` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the attribute macro `::pgrx::pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0521]: borrowed data escapes outside of function
--> tests/compile-fail/aggregate-functions-dont-run-forever.rs:113:1
Expand All @@ -48,7 +48,7 @@ error[E0521]: borrowed data escapes outside of function
| lifetime `'fcx` defined here
| argument requires that `'fcx` must outlive `'static`
|
= note: this error originates in the attribute macro `pg_aggregate` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the attribute macro `::pgrx::pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0521]: borrowed data escapes outside of function
--> tests/compile-fail/aggregate-functions-dont-run-forever.rs:141:1
Expand All @@ -61,4 +61,4 @@ error[E0521]: borrowed data escapes outside of function
| lifetime `'fcx` defined here
| argument requires that `'fcx` must outlive `'static`
|
= note: this error originates in the attribute macro `pg_aggregate` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the attribute macro `::pgrx::pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
4 changes: 2 additions & 2 deletions pgrx-tests/tests/compile-fail/eq-for-postgres_hash.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ note: required by a bound in `brokentype_hash`
| ^^^^^^^^^^^^ required by this bound in `brokentype_hash`
5 | pub struct BrokenType {
| ---------- required by a bound in this function
= note: this error originates in the derive macro `PostgresHash` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the attribute macro `::pgrx::pgrx_macros::pg_extern` which comes from the expansion of the derive macro `PostgresHash` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider annotating `BrokenType` with `#[derive(Hash)]`
|
5 + #[derive(Hash)]
Expand All @@ -59,7 +59,7 @@ note: required by a bound in `brokentype_hash`
| ^^^^^^^^^^^^ required by this bound in `brokentype_hash`
5 | pub struct BrokenType {
| ---------- required by a bound in this function
= note: this error originates in the derive macro `PostgresHash` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the attribute macro `::pgrx::pgrx_macros::pg_extern` which comes from the expansion of the derive macro `PostgresHash` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider annotating `BrokenType` with `#[derive(Eq)]`
|
5 + #[derive(Eq)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ error[E0521]: borrowed data escapes outside of function
--> tests/compile-fail/postgres-strings-arent-immortal.rs:4:67
|
3 | #[pg_extern]
| ------------ lifetime `'fcx` defined here
| ------------
| |
| lifetime `'fcx` defined here
| in this procedural macro expansion
4 | fn split(input: &'static str, pattern: &str) -> Vec<&'static str> {
| ___________________________________________________________________^
5 | | input.split_terminator(pattern).collect()
Expand All @@ -12,3 +15,5 @@ error[E0521]: borrowed data escapes outside of function
| | `fcinfo` is a reference that is only valid in the function body
| |_`fcinfo` escapes the function body here
| argument requires that `'fcx` must outlive `'static`
|
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ error[E0521]: borrowed data escapes outside of function
--> tests/compile-fail/table-iterators-arent-immortal.rs:6:78
|
3 | #[pg_extern]
| ------------ lifetime `'fcx` defined here
| ------------
| |
| lifetime `'fcx` defined here
| in this procedural macro expansion
...
6 | ) -> TableIterator<(name!(a, &'static str), name!(b, Option<&'static str>))> {
| ______________________________________________________________________________^
Expand All @@ -13,3 +16,5 @@ error[E0521]: borrowed data escapes outside of function
| | `fcinfo` is a reference that is only valid in the function body
| |_`fcinfo` escapes the function body here
| argument requires that `'fcx` must outlive `'static`
|
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ note: required by a bound in `brokentype_eq`
| ^^^^^^^^^^ required by this bound in `brokentype_eq`
5 | pub struct BrokenType {
| ---------- required by a bound in this function
= note: this error originates in the derive macro `PostgresEq` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the attribute macro `::pgrx::pgrx_macros::pg_operator` which comes from the expansion of the derive macro `PostgresEq` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider annotating `BrokenType` with `#[derive(Eq)]`
|
5 + #[derive(Eq)]
Expand Down
4 changes: 4 additions & 0 deletions pgrx-tests/tests/todo/busted-exotic-signature.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
error[E0277]: the trait bound `Vec<Option<PgHeapTuple<'_, AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
--> tests/todo/busted-exotic-signature.rs:12:9
|
10 | #[pg_extern]
| ------------ in this procedural macro expansion
11 | fn exotic_signature(
12 | _cats: pgrx::default!(
| ^^^^^ the trait `ArgAbi<'_>` is not implemented for `Vec<Option<PgHeapTuple<'_, AllocatedByRust>>>`, which is required by `Option<Vec<Option<PgHeapTuple<'_, AllocatedByRust>>>>: ArgAbi<'_>`
|
Expand All @@ -14,3 +17,4 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
|
| pub unsafe fn next_arg_unchecked<T: ArgAbi<'fcx>>(&mut self) -> Option<T> {
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
12 changes: 12 additions & 0 deletions pgrx-tests/tests/todo/composite-types-broken-on-spi.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
error[E0277]: the trait bound `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
--> tests/todo/composite-types-broken-on-spi.rs:60:9
|
58 | #[pg_extern]
| ------------ in this procedural macro expansion
59 | fn sum_scritches_for_names_strict_optional_items(
60 | dogs: Vec<Option<pgrx::composite_type!("Dog")>>,
| ^^^^ the trait `ArgAbi<'_>` is not implemented for `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>`
|
Expand All @@ -13,10 +16,14 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
|
| pub unsafe fn next_arg_unchecked<T: ArgAbi<'fcx>>(&mut self) -> Option<T> {
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
--> tests/todo/composite-types-broken-on-spi.rs:77:9
|
75 | #[pg_extern]
| ------------ in this procedural macro expansion
76 | fn sum_scritches_for_names_default_optional_items(
77 | dogs: pgrx::default!(
| ^^^^ the trait `ArgAbi<'_>` is not implemented for `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>`
|
Expand All @@ -29,10 +36,14 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
|
| pub unsafe fn next_arg_unchecked<T: ArgAbi<'fcx>>(&mut self) -> Option<T> {
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
--> tests/todo/composite-types-broken-on-spi.rs:97:9
|
95 | #[pg_extern]
| ------------ in this procedural macro expansion
96 | fn sum_scritches_for_names_optional_items(
97 | dogs: Option<Vec<Option<pgrx::composite_type!("Dog")>>>,
| ^^^^ the trait `ArgAbi<'_>` is not implemented for `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>`, which is required by `Option<Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>>: ArgAbi<'_>`
|
Expand All @@ -46,6 +57,7 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
|
| pub unsafe fn next_arg_unchecked<T: ArgAbi<'fcx>>(&mut self) -> Option<T> {
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` is not an iterator
--> tests/todo/composite-types-broken-on-spi.rs:125:20
Expand Down
4 changes: 4 additions & 0 deletions pgrx-tests/tests/todo/random-vec-strs-arent-okay.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
error[E0277]: the trait bound `Vec<Option<&str>>: ArgAbi<'_>` is not satisfied
--> tests/todo/random-vec-strs-arent-okay.rs:8:5
|
5 | #[pg_extern]
| ------------ in this procedural macro expansion
...
8 | args: default!(Vec<Option<&'a str>>, "ARRAY[]::text[]"),
| ^^^^ the trait `ArgAbi<'_>` is not implemented for `Vec<Option<&str>>`
|
Expand All @@ -13,3 +16,4 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
|
| pub unsafe fn next_arg_unchecked<T: ArgAbi<'fcx>>(&mut self) -> Option<T> {
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
13 changes: 11 additions & 2 deletions pgrx-tests/tests/todo/roundtrip-tests.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ error[E0261]: use of undeclared lifetime name `'a`
69 | Vec<Option<&'a str>>,
| ^^ undeclared lifetime

error[E0261]: use of undeclared lifetime name `'a`
--> tests/todo/roundtrip-tests.rs:69:21
|
67 | rt_array_refstr,
| - lifetime `'a` is missing in item created through this procedural macro
68 | test_rt_array_refstr,
69 | Vec<Option<&'a str>>,
| ^^ undeclared lifetime

error[E0261]: use of undeclared lifetime name `'a`
--> tests/todo/roundtrip-tests.rs:69:21
|
Expand Down Expand Up @@ -62,7 +71,7 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
|
| pub unsafe fn next_arg_unchecked<T: ArgAbi<'fcx>>(&mut self) -> Option<T> {
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
= note: this error originates in the macro `roundtrip` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the attribute macro `pg_extern` which comes from the expansion of the macro `roundtrip` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Vec<Option<&[u8]>>: FromDatum` is not satisfied
--> tests/todo/roundtrip-tests.rs:36:38
Expand Down Expand Up @@ -147,7 +156,7 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
|
| pub unsafe fn next_arg_unchecked<T: ArgAbi<'fcx>>(&mut self) -> Option<T> {
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
= note: this error originates in the macro `roundtrip` (in Nightly builds, run with -Z macro-backtrace for more info)
= note: this error originates in the attribute macro `pg_extern` which comes from the expansion of the macro `roundtrip` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Vec<Option<&CStr>>: FromDatum` is not satisfied
--> tests/todo/roundtrip-tests.rs:36:38
Expand Down
Loading

0 comments on commit 2c1d388

Please sign in to comment.