Skip to content

Commit fd0a466

Browse files
Use diagnostic attrs for ABI/SQL-related traits (pgcentralfoundation#2203)
We use the attributes to hide `FunctionMetadata` from diagnostic output, as it is a "behind the scenes" trait, and emphasize `SqlTranslatable`. We also comment briefly on why `ArgAbi<'_>` and `RetAbi` are like they are. This significantly improves the diagnostics for all of these by massively cutting down on noise generated while flailing around.
1 parent 15ce9ea commit fd0a466

10 files changed

Lines changed: 110 additions & 10 deletions

pgrx-sql-entity-graph/src/metadata/function_metadata.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ pub trait FunctionMetadata<A> {
4242

4343
macro_rules! impl_fn {
4444
($($A:ident),* $(,)?) => {
45+
#[diagnostic::do_not_recommend]
4546
impl<$($A,)* R, F> FunctionMetadata<($($A,)*)> for F
4647
where
4748
$($A: SqlTranslatable,)*
@@ -56,6 +57,8 @@ macro_rules! impl_fn {
5657
}
5758
}
5859
}
60+
61+
#[diagnostic::do_not_recommend]
5962
impl<$($A,)* R> FunctionMetadata<($($A,)*)> for unsafe fn($($A,)*) -> R
6063
where
6164
$($A: SqlTranslatable,)*

pgrx-sql-entity-graph/src/metadata/sql_translatable.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ or the Rust handling in PGRX may emit undefined behavior.
7070
It cannot be made private or sealed due to details of the structure of the PGRX framework.
7171
Nonetheless, if you are not confident the translation is valid: do not implement this trait.
7272
*/
73+
#[diagnostic::on_unimplemented(
74+
message = "`{Self}` has no representation in SQL",
75+
label = "non-SQL type"
76+
)]
7377
pub unsafe trait SqlTranslatable {
7478
fn type_name() -> &'static str {
7579
core::any::type_name::<Self>()
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
use pgrx::prelude::*;
2+
3+
fn main() {}
4+
5+
#[pg_extern]
6+
pub fn whatever(a: u32) -> u32 {
7+
a
8+
}
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
error[E0277]: the trait bound `fn(u32) -> u32: FunctionMetadata<_>` is not satisfied
2+
--> tests/compile-fail/u32.rs:6:5
3+
|
4+
5 | #[pg_extern]
5+
| ------------ in this procedural macro expansion
6+
6 | pub fn whatever(a: u32) -> u32 {
7+
| ^^ the trait `FunctionMetadata<_>` is not implemented for `fn(u32) -> u32`
8+
|
9+
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
10+
11+
error[E0599]: `u32` has no representation in SQL
12+
--> tests/compile-fail/u32.rs:5:1
13+
|
14+
5 | #[pg_extern]
15+
| ^^^^^^^^^^^^ non-SQL type
16+
|
17+
= note: the following trait bounds were not satisfied:
18+
`u32: SqlTranslatable`
19+
which is required by `&u32: SqlTranslatable`
20+
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
21+
22+
error[E0277]: the trait bound `u32: RetAbi` is not satisfied
23+
--> tests/compile-fail/u32.rs:6:28
24+
|
25+
6 | pub fn whatever(a: u32) -> u32 {
26+
| ^^^ the trait `BoxRet` is not implemented for `u32`
27+
|
28+
= help: the following other types implement trait `BoxRet`:
29+
f32
30+
f64
31+
i16
32+
i32
33+
i64
34+
i8
35+
= note: required for `u32` to implement `RetAbi`
36+
37+
error[E0277]: `u32` cannot be passed into a Postgres function as a Datum
38+
--> tests/compile-fail/u32.rs:6:17
39+
|
40+
5 | #[pg_extern]
41+
| ------------ in this procedural macro expansion
42+
6 | pub fn whatever(a: u32) -> u32 {
43+
| ^ the trait `ArgAbi<'_>` is not implemented for `u32`
44+
|
45+
= help: the following other types implement trait `ArgAbi<'fcx>`:
46+
f32
47+
f64
48+
i16
49+
i32
50+
i64
51+
i8
52+
note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_unchecked`
53+
--> $WORKSPACE/pgrx/src/callconv.rs
54+
|
55+
| pub unsafe fn next_arg_unchecked<T: ArgAbi<'fcx>>(&mut self) -> Option<T> {
56+
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
57+
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
58+
59+
error[E0277]: the trait bound `u32: RetAbi` is not satisfied
60+
--> tests/compile-fail/u32.rs:6:32
61+
|
62+
5 | #[pg_extern]
63+
| ------------ in this procedural macro expansion
64+
6 | pub fn whatever(a: u32) -> u32 {
65+
| ________________________________^
66+
7 | | a
67+
8 | | }
68+
| |_^ the trait `BoxRet` is not implemented for `u32`
69+
|
70+
= help: the following other types implement trait `BoxRet`:
71+
f32
72+
f64
73+
i16
74+
i32
75+
i64
76+
i8
77+
= note: required for `u32` to implement `RetAbi`
78+
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)

pgrx-tests/tests/todo/busted-exotic-signature.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error[E0277]: the trait bound `Vec<Option<PgHeapTuple<'_, AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
1+
error[E0277]: `Vec<Option<PgHeapTuple<'_, AllocatedByRust>>>` cannot be passed into a Postgres function as a Datum
22
--> tests/todo/busted-exotic-signature.rs:12:9
33
|
44
10 | #[pg_extern]

pgrx-tests/tests/todo/composite-types-broken-on-spi.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error[E0277]: the trait bound `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
1+
error[E0277]: `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>` cannot be passed into a Postgres function as a Datum
22
--> tests/todo/composite-types-broken-on-spi.rs:60:9
33
|
44
58 | #[pg_extern]
@@ -18,7 +18,7 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
1818
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
1919
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
2020

21-
error[E0277]: the trait bound `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
21+
error[E0277]: `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>` cannot be passed into a Postgres function as a Datum
2222
--> tests/todo/composite-types-broken-on-spi.rs:77:9
2323
|
2424
75 | #[pg_extern]
@@ -38,7 +38,7 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
3838
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
3939
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
4040

41-
error[E0277]: the trait bound `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
41+
error[E0277]: `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>` cannot be passed into a Postgres function as a Datum
4242
--> tests/todo/composite-types-broken-on-spi.rs:97:9
4343
|
4444
95 | #[pg_extern]

pgrx-tests/tests/todo/random-vec-strs-arent-okay.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error[E0277]: the trait bound `Vec<Option<&str>>: ArgAbi<'_>` is not satisfied
1+
error[E0277]: `Vec<Option<&str>>` cannot be passed into a Postgres function as a Datum
22
--> tests/todo/random-vec-strs-arent-okay.rs:8:5
33
|
44
5 | #[pg_extern]

pgrx-tests/tests/todo/roundtrip-tests.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ help: consider introducing lifetime `'a` here
5555
68 | test_rt_array_refstr<'a>,
5656
| ++++
5757

58-
error[E0277]: the trait bound `Vec<Option<&[u8]>>: ArgAbi<'_>` is not satisfied
58+
error[E0277]: `Vec<Option<&[u8]>>` cannot be passed into a Postgres function as a Datum
5959
--> tests/todo/roundtrip-tests.rs:22:23
6060
|
6161
22 | fn $fname(i: $rtype) -> $rtype {
@@ -138,7 +138,7 @@ note: required by a bound in `pgrx::Spi::get_one_with_args`
138138
| ^^^^^^^^^ required by this bound in `Spi::get_one_with_args`
139139
= note: this error originates in the macro `roundtrip_test` which comes from the expansion of the macro `roundtrip` (in Nightly builds, run with -Z macro-backtrace for more info)
140140

141-
error[E0277]: the trait bound `Vec<Option<&CStr>>: ArgAbi<'_>` is not satisfied
141+
error[E0277]: `Vec<Option<&CStr>>` cannot be passed into a Postgres function as a Datum
142142
--> tests/todo/roundtrip-tests.rs:22:23
143143
|
144144
22 | fn $fname(i: $rtype) -> $rtype {

pgrx-tests/tests/todo/vec-option-composite_type-nesting-problems.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error[E0277]: the trait bound `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
1+
error[E0277]: `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>` cannot be passed into a Postgres function as a Datum
22
--> tests/todo/vec-option-composite_type-nesting-problems.rs:55:5
33
|
44
53 | #[pg_extern]
@@ -18,7 +18,7 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
1818
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
1919
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
2020

21-
error[E0277]: the trait bound `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
21+
error[E0277]: `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>` cannot be passed into a Postgres function as a Datum
2222
--> tests/todo/vec-option-composite_type-nesting-problems.rs:72:5
2323
|
2424
70 | #[pg_extern]
@@ -38,7 +38,7 @@ note: required by a bound in `pgrx::callconv::Args::<'a, 'fcx>::next_arg_uncheck
3838
| ^^^^^^^^^^^^ required by this bound in `Args::<'a, 'fcx>::next_arg_unchecked`
3939
= note: this error originates in the attribute macro `pg_extern` (in Nightly builds, run with -Z macro-backtrace for more info)
4040

41-
error[E0277]: the trait bound `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>: ArgAbi<'_>` is not satisfied
41+
error[E0277]: `Vec<Option<pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>>` cannot be passed into a Postgres function as a Datum
4242
--> tests/todo/vec-option-composite_type-nesting-problems.rs:89:5
4343
|
4444
87 | #[pg_extern]

pgrx/src/callconv.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ static VIRTUAL_ARGUMENT: ReadOnly<pg_sys::NullableDatum> =
6060
/// This trait is exposed to external code so macro-generated wrapper fn may expand to calls to it.
6161
/// The number of invariants implementers must uphold is unlikely to be adequately documented.
6262
/// Prefer to use ArgAbi as a trait bound instead of implementing it, or even calling it, yourself.
63+
#[diagnostic::on_unimplemented(
64+
message = "`{Self}` cannot be passed into a Postgres function as a Datum"
65+
)]
6366
pub unsafe trait ArgAbi<'fcx>: Sized {
6467
/// Unbox an argument with a matching type.
6568
///
@@ -378,6 +381,10 @@ pub unsafe trait RetAbi: Sized {
378381
/// Postgres functions, pgrx uses a very complicated trait, RetAbi. In practice, however, most
379382
/// types do not need to think about its many sharp-edged cases. Instead, they should implement
380383
/// this simplified trait, BoxRet. A blanket impl of RetAbi for BoxRet takes care of the rest.
384+
#[diagnostic::on_unimplemented(
385+
message = "{Self} cannot be returned from a Postgres function as a Datum",
386+
note = "implement BoxRet instead if possible"
387+
)]
381388
pub unsafe trait BoxRet: Sized {
382389
/// # Safety
383390
/// You have to actually return the resulting Datum to the function.

0 commit comments

Comments
 (0)