public inbox for [email protected]
help / color / mirror / Atom feed[[deprecated("don't call this, call that")]]
4+ messages / 4 participants
[nested] [flat]
* [[deprecated("don't call this, call that")]]
@ 2026-04-09 03:34 Thomas Munro <[email protected]>
0 siblings, 2 replies; 4+ messages in thread
From: Thomas Munro @ 2026-04-09 03:34 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi,
While working on 1e7fe06c, I wished I could make functions generate
compiler warnings:
pg_attribute_deprecated("use pg_mblen_{cstr,range,with_len,unbounded} instead")
extern int pg_mblen(const char *mbstr);
That'd avoid accidental reintroduction, and also get extension
maintainers' attention. $SUBJECT is C23/C++14's syntax, but you've
long been able to do that with in __attribute__ or __declspec for the
usual suspects so I looked into which compiler versions introduced
that and came up with the attached.
The idea would be to back-patch the deprecation warnings, and delete
the functions in, I guess now, v20. Then the deprecation notice
facility would always be there for next time we need it.
Attachments:
[text/x-patch] 0001-Provide-pg_attribute_deprecated-message-macro.patch (1.6K, 2-0001-Provide-pg_attribute_deprecated-message-macro.patch)
download | inline diff:
From b780ba45bbb366406cd2ff42db3c69070ca2b22e Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Wed, 1 Apr 2026 22:20:55 +1300
Subject: [PATCH 1/3] Provide pg_attribute_deprecated("message") macro.
This expands to C23/C++14 [[deprecated("message")]], or equivalent
attributes available since GCC 4.9, Clang 2.9 and MSVC 2008 (and further
back without a message). It can be placed before a type or function
declaration to trigger compiler warnings when used.
Backpatch-through: 14
---
src/include/c.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/include/c.h b/src/include/c.h
index 88d13ec9993..67a759b1bf3 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -238,6 +238,21 @@ extern "C++"
#define pg_attribute_target(...)
#endif
+/*
+ * Support for marking functions and types as deprecated, with a compiler
+ * warning if the function is used. Precedes a declaration.
+ */
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L) || \
+ (defined(__cplusplus) && __cplusplus >= 201402L)
+#define pg_attribute_deprecated(message) [[deprecated(message)]]
+#elif defined(__clang__) || (defined(__GNUC__) && __GNUC__ >= 5)
+#define pg_attribute_deprecated(message) __attribute__((deprecated(message)))
+#elif defined(_MSC_VER)
+#define pg_attribute_deprecated(message) __declspec(deprecated(message))
+#else
+#define pg_attribute_deprecated(message)
+#endif
+
/*
* Append PG_USED_FOR_ASSERTS_ONLY to definitions of variables that are only
* used in assert-enabled builds, to avoid compiler warnings about unused
--
2.53.0
[text/x-patch] 0002-Mark-pg_mblen-and-t_is-as-deprecated.patch (1.7K, 3-0002-Mark-pg_mblen-and-t_is-as-deprecated.patch)
download | inline diff:
From 57badb2c9b3363ac2f10dd4df222b2bb6c405e82 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Wed, 1 Apr 2026 22:28:25 +1300
Subject: [PATCH 2/3] Mark pg_mblen() and t_is*() as deprecated.
These functions were deprecated by commit 1e7fe06c and shouldn't be used
in new code. Adding a deprecated attribute will cause warnings in
extension code that is using them.
Backpatch-through: 14
Reviewed-by:
Discussion:
---
src/include/mb/pg_wchar.h | 2 +-
src/include/tsearch/ts_locale.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h
index e1655fe61d6..a3326e365a7 100644
--- a/src/include/mb/pg_wchar.h
+++ b/src/include/mb/pg_wchar.h
@@ -700,7 +700,7 @@ extern int pg_mblen_range(const char *mbstr, const char *end);
extern int pg_mblen_with_len(const char *mbstr, int limit);
extern int pg_mblen_unbounded(const char *mbstr);
-/* deprecated */
+pg_attribute_deprecated("use pg_mblen_{cstr,range,with_len,unbounded} instead")
extern int pg_mblen(const char *mbstr);
extern int pg_dsplen(const char *mbstr);
diff --git a/src/include/tsearch/ts_locale.h b/src/include/tsearch/ts_locale.h
index 6e2d67ee4a5..90e533eea31 100644
--- a/src/include/tsearch/ts_locale.h
+++ b/src/include/tsearch/ts_locale.h
@@ -60,7 +60,7 @@ extern int t_is##character_class##_with_len(const char *ptr, int len); \
extern int t_is##character_class##_cstr(const char *ptr); \
extern int t_is##character_class##_unbounded(const char *ptr); \
\
-/* deprecated */ \
+pg_attribute_deprecated("use t_isXXX_{cstr,with_len,unbounded} instead") \
extern int t_is##character_class(const char *ptr);
GENERATE_T_ISCLASS_DECL(alnum);
--
2.53.0
[text/x-patch] 0003-Remove-pg_mblen-and-related-functions.patch (3.5K, 4-0003-Remove-pg_mblen-and-related-functions.patch)
download | inline diff:
From 477cf8ec1ae9ef333988c0dcd48a20d00eacd93b Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Wed, 1 Apr 2026 23:24:33 +1300
Subject: [PATCH 3/3] Remove pg_mblen() and related functions.
pg_mblen() was unsafe and declared deprecated in all release branches,
and can now removed in master. The t_is*() functions were less unsafe,
but needed to know which pg_mblen_XXX() replacement to call so they also
gained parallel replacements.
Reviewed-by:
Discussion:
---
src/backend/tsearch/ts_locale.c | 12 ------------
src/backend/utils/mb/mbutils.c | 10 ----------
src/include/mb/pg_wchar.h | 3 ---
src/include/tsearch/ts_locale.h | 9 +--------
4 files changed, 1 insertion(+), 33 deletions(-)
diff --git a/src/backend/tsearch/ts_locale.c b/src/backend/tsearch/ts_locale.c
index df02ffb12fd..12ef3d11b32 100644
--- a/src/backend/tsearch/ts_locale.c
+++ b/src/backend/tsearch/ts_locale.c
@@ -41,18 +41,6 @@ int \
t_is##character_class##_cstr(const char *ptr) \
{ \
return t_is##character_class##_with_len(ptr, pg_mblen_cstr(ptr)); \
-} \
-/* ptr shall point to a string with pre-validated encoding */ \
-int \
-t_is##character_class##_unbounded(const char *ptr) \
-{ \
- return t_is##character_class##_with_len(ptr, pg_mblen_unbounded(ptr)); \
-} \
-/* historical name for _unbounded */ \
-int \
-t_is##character_class(const char *ptr) \
-{ \
- return t_is##character_class##_unbounded(ptr); \
}
GENERATE_T_ISCLASS_DEF(alnum)
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 78f4d5e202c..08d80f2c36f 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -1143,16 +1143,6 @@ pg_mblen_unbounded(const char *mbstr)
return length;
}
-/*
- * Historical name for pg_mblen_unbounded(). Should not be used and will be
- * removed in a later version.
- */
-int
-pg_mblen(const char *mbstr)
-{
- return pg_mblen_unbounded(mbstr);
-}
-
/* returns the display length of a multibyte character */
int
pg_dsplen(const char *mbstr)
diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h
index a3326e365a7..e42cf562635 100644
--- a/src/include/mb/pg_wchar.h
+++ b/src/include/mb/pg_wchar.h
@@ -700,9 +700,6 @@ extern int pg_mblen_range(const char *mbstr, const char *end);
extern int pg_mblen_with_len(const char *mbstr, int limit);
extern int pg_mblen_unbounded(const char *mbstr);
-pg_attribute_deprecated("use pg_mblen_{cstr,range,with_len,unbounded} instead")
-extern int pg_mblen(const char *mbstr);
-
extern int pg_dsplen(const char *mbstr);
extern int pg_mbstrlen(const char *mbstr);
extern int pg_mbstrlen_with_len(const char *mbstr, int limit);
diff --git a/src/include/tsearch/ts_locale.h b/src/include/tsearch/ts_locale.h
index 90e533eea31..b6a2d29ee3f 100644
--- a/src/include/tsearch/ts_locale.h
+++ b/src/include/tsearch/ts_locale.h
@@ -52,16 +52,9 @@ ts_copychar_cstr(void *dest, const void *src)
return ts_copychar_with_len(dest, src, pg_mblen_cstr((const char *) src));
}
-/* Historical macro for the above. */
-#define COPYCHAR ts_copychar_cstr
-
#define GENERATE_T_ISCLASS_DECL(character_class) \
extern int t_is##character_class##_with_len(const char *ptr, int len); \
-extern int t_is##character_class##_cstr(const char *ptr); \
-extern int t_is##character_class##_unbounded(const char *ptr); \
-\
-pg_attribute_deprecated("use t_isXXX_{cstr,with_len,unbounded} instead") \
-extern int t_is##character_class(const char *ptr);
+extern int t_is##character_class##_cstr(const char *ptr);
GENERATE_T_ISCLASS_DECL(alnum);
GENERATE_T_ISCLASS_DECL(alpha);
--
2.53.0
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [[deprecated("don't call this, call that")]]
@ 2026-04-09 03:42 Michael Paquier <[email protected]>
parent: Thomas Munro <[email protected]>
1 sibling, 1 reply; 4+ messages in thread
From: Michael Paquier @ 2026-04-09 03:42 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Thu, Apr 09, 2026 at 03:34:30PM +1200, Thomas Munro wrote:
> The idea would be to back-patch the deprecation warnings, and delete
> the functions in, I guess now, v20. Then the deprecation notice
> facility would always be there for next time we need it.
That would be a nice addition, especially with the recent mblen()
business (could have used that a few times myself). So +1 for the
addition of the macro in the back branches.
Do we need to be that conservative with the removal in v19? We could
just pull the deletion switch without waiting for v20, IMO.. With the
deprecated macro in place, at least folks would be aware of the
problem.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [[deprecated("don't call this, call that")]]
@ 2026-04-09 10:42 Peter Eisentraut <[email protected]>
parent: Thomas Munro <[email protected]>
1 sibling, 0 replies; 4+ messages in thread
From: Peter Eisentraut @ 2026-04-09 10:42 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; PostgreSQL Hackers <[email protected]>
On 09.04.26 05:34, Thomas Munro wrote:
> While working on 1e7fe06c, I wished I could make functions generate
> compiler warnings:
>
> pg_attribute_deprecated("use pg_mblen_{cstr,range,with_len,unbounded} instead")
> extern int pg_mblen(const char *mbstr);
>
> That'd avoid accidental reintroduction, and also get extension
> maintainers' attention. $SUBJECT is C23/C++14's syntax, but you've
> long been able to do that with in __attribute__ or __declspec for the
> usual suspects so I looked into which compiler versions introduced
> that and came up with the attached.
Yes, this makes sense. There have been discussions about a deprecated
attribute before (such as [0]), but in those cases it was mostly about
nagging people about coding style, which had potentially annoying
effects for extension authors that want to cover many major versions.
But in this case, we are actively encouraging people to get rid of a
function use, so it seems like a very suitable use case.
[0]:
https://www.postgresql.org/message-id/d80b6adf-4bfd-4172-a9cd-2ad6e23b1a08%40eisentraut.org
> The idea would be to back-patch the deprecation warnings, and delete
> the functions in, I guess now, v20. Then the deprecation notice
> facility would always be there for next time we need it.
It might be enough to put the deprecation attribute into PG19. That
way, extension authors will see it (unless the extension is not
supporting PG19, in which case it seems likely that they also won't
bother to make any adjustments to avoid deprecated functions).
This piece
+#elif defined(__clang__) || (defined(__GNUC__) && __GNUC__ >= 5)
+#define pg_attribute_deprecated(message)
__attribute__((deprecated(message)))
could be written as
#elif __has_attribute(deprecated)
...
__has_attribute works back to PG14.
If you do want to backpatch it, I would skip the C23/C++ branch for
versions PG18 and older. That way, each branch has an internally
consistent handling of attributes. (We have some C23/C++ attributes in
PG19, but not before.)
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [[deprecated("don't call this, call that")]]
@ 2026-04-09 11:31 Bertrand Drouvot <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Bertrand Drouvot @ 2026-04-09 11:31 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Thomas Munro <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On Thu, Apr 09, 2026 at 12:42:36PM +0900, Michael Paquier wrote:
> On Thu, Apr 09, 2026 at 03:34:30PM +1200, Thomas Munro wrote:
> > The idea would be to back-patch the deprecation warnings, and delete
> > the functions in, I guess now, v20. Then the deprecation notice
> > facility would always be there for next time we need it.
>
> That would be a nice addition, especially with the recent mblen()
> business (could have used that a few times myself). So +1 for the
> addition of the macro in the back branches.
>
> Do we need to be that conservative with the removal in v19? We could
> just pull the deletion switch without waiting for v20, IMO.. With the
> deprecated macro in place, at least folks would be aware of the
> problem.
+1 on the idea. FWIW it has also been proposed in [1]. Also good candidates are
XLogRecPtrIsInvalid() and StaticAssertStmt(). Note that a recent commit made
use of StaticAssertStmt(), see [2].
[1]: https://postgr.es/m/[email protected]
[2]: https://postgr.es/m/adeNWH5pDawDvvR2%40ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-04-09 11:31 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-09 03:34 [[deprecated("don't call this, call that")]] Thomas Munro <[email protected]>
2026-04-09 03:42 ` Michael Paquier <[email protected]>
2026-04-09 11:31 ` Bertrand Drouvot <[email protected]>
2026-04-09 10:42 ` Peter Eisentraut <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox