Build fix for Tcl 9.0 (while remaining compatible with Tcl 8.4-8.6) - #57
Open
devrimgunduz wants to merge 1 commit into
Open
Build fix for Tcl 9.0 (while remaining compatible with Tcl 8.4-8.6)#57devrimgunduz wants to merge 1 commit into
devrimgunduz wants to merge 1 commit into
Conversation
Problem
-------
pgtcl fails to compile against Tcl 9.0 headers (e.g. building against
PostgreSQL 19 / recent Fedora-Rawhide-style toolchains that ship Tcl 9).
The build aborts with dozens of
error: expected ';', ',' or ')' before 'objv'
and, once that is fixed, a second wave of
error: passing argument N of '...' from incompatible pointer type
note: expected 'Tcl_Size *' but argument is of type 'int *'
and one instance of
error: storage size of 'cmd_info' isn't known
After those are resolved, the build succeeds but leaves a small number of
warnings, also fixed by this patch (see item 6 below).
Root cause
----------
Tcl 9.0 removed two long-deprecated Tcl-8.0-era compatibility shims that
pgtcl's sources still relied on:
1. The `CONST` / `CONST84` / `CONST86` macros are gone. Every
`Tcl_Obj *CONST objv[]` (and one `CONST char *`) in pgtcl no longer
parses, since `CONST` is now just an unknown identifier sitting where
the compiler expects a parameter name. Plain `const` has been the
correct, portable spelling since Tcl 8.4, so this patch simply spells
it that way everywhere.
2. String/list lengths and element counts that used to be returned via
"int *" out-parameters (Tcl_GetStringFromObj, Tcl_GetByteArrayFromObj,
Tcl_ListObjGetElements, Tcl_ListObjLength, ...) now use the new,
pointer-width `Tcl_Size` type. Every local variable, struct field and
helper-function parameter in pgtcl that receives one of these lengths
has been changed to `Tcl_Size`, with a small compatibility header
(generic/pgtclCompat.h) providing a `typedef int Tcl_Size;` fallback
for Tcl 8.x, which doesn't define this type at all. This mirrors the
migration path documented by the Tcl maintainers:
https://www.tcl-lang.org/doc/howto/size_t.html
3. `struct Tcl_CmdInfo` is used in two places in pgtclSqlite.c, but
`Tcl_CmdInfo` has always actually been a typedef, not a tagged
struct; older Tcl headers happened to also declare a struct tag of
the same name; the anonymous-struct-only header in newer Tcl no
longer does, so the `struct` keyword must be dropped.
4. One additional, previously-hidden bug surfaces once the above parse
errors are fixed: Pg_lo_read() passed the long-obsolete
`TCL_PARSE_PART1` flag to Tcl_ObjSetVar2(). That flag was removed in
Tcl 9.0. Tcl_ObjSetVar2() has parsed array syntax ("name(index)") in
part1 automatically for a very long time regardless of this flag, so
it is simply dropped; behaviour is unchanged on every supported Tcl
version.
5. A handful of `ckfree(some_const_char_ptr)` calls relied on the old
`ckfree(x)` macro casting its argument to `(char *)` internally.
Tcl 9.0's non-debug `ckfree` is a bare `#define ckfree Tcl_Free`
with no cast, so these now need an explicit `(void *)` cast at the
call site to avoid a "discards const qualifier" warning.
6. Three warnings remained after the above (all pre-existing, just
never previously reached because the build died earlier):
- pgtclId.c: getresid() and PgGetConnByResultId() take a
`const char *` handle string, then temporarily NUL the last '.'
in place (and restore it) to split the connection id from the
result id. `mark = strrchr(id, '.')` assigned the result
straight into a `char *`, which some toolchains flag as
discarding the const qualifier since `id` is const. The
temporary in-place mutation itself is unchanged (it's the
existing, intentional behaviour); the cast is now explicit:
`mark = (char *) strrchr(id, '.');`
- pgtclCmds.c: Pg_select() declared `int tuplesProcessed = 0;`
and incremented it once per row, but never read it anywhere.
The `-count var` feature that this looks like it was meant to
feed is actually driven by a separate `numTuples`/`tuplesVarObj`
pair a few lines away, so `tuplesProcessed` was simply dead
code left over from an earlier version. It has been removed
(declaration + increment); this does not change any observable
behaviour, since its value was never used.
None of this changes pgtcl's behaviour on any currently supported Tcl
version -- it only replaces removed/renamed compatibility spellings with
their modern equivalents, and drops one genuinely-unused variable.
Hacked by Claude, tested by me
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
pgtcl fails to compile against Tcl 9.0 headers (e.g. building against PostgreSQL 19 / recent Fedora-Rawhide-style toolchains that ship Tcl 9). The build aborts with dozens of
and, once that is fixed, a second wave of
and one instance of
After those are resolved, the build succeeds but leaves a small number of warnings, also fixed by this patch (see item 6 below).
Root cause
Tcl 9.0 removed two long-deprecated Tcl-8.0-era compatibility shims that pgtcl's sources still relied on:
The
CONST/CONST84/CONST86macros are gone. EveryTcl_Obj *CONST objv[](and oneCONST char *) in pgtcl no longer parses, sinceCONSTis now just an unknown identifier sitting where the compiler expects a parameter name. Plainconsthas been the correct, portable spelling since Tcl 8.4, so this patch simply spells it that way everywhere.String/list lengths and element counts that used to be returned via "int *" out-parameters (Tcl_GetStringFromObj, Tcl_GetByteArrayFromObj, Tcl_ListObjGetElements, Tcl_ListObjLength, ...) now use the new, pointer-width
Tcl_Sizetype. Every local variable, struct field and helper-function parameter in pgtcl that receives one of these lengths has been changed toTcl_Size, with a small compatibility header (generic/pgtclCompat.h) providing atypedef int Tcl_Size;fallback for Tcl 8.x, which doesn't define this type at all. This mirrors the migration path documented by the Tcl maintainers: https://www.tcl-lang.org/doc/howto/size_t.htmlstruct Tcl_CmdInfois used in two places in pgtclSqlite.c, butTcl_CmdInfohas always actually been a typedef, not a tagged struct; older Tcl headers happened to also declare a struct tag of the same name; the anonymous-struct-only header in newer Tcl no longer does, so thestructkeyword must be dropped.One additional, previously-hidden bug surfaces once the above parse errors are fixed: Pg_lo_read() passed the long-obsolete
TCL_PARSE_PART1flag to Tcl_ObjSetVar2(). That flag was removed in Tcl 9.0. Tcl_ObjSetVar2() has parsed array syntax ("name(index)") in part1 automatically for a very long time regardless of this flag, so it is simply dropped; behaviour is unchanged on every supported Tcl version.A handful of
ckfree(some_const_char_ptr)calls relied on the oldckfree(x)macro casting its argument to(char *)internally. Tcl 9.0's non-debugckfreeis a bare#define ckfree Tcl_Freewith no cast, so these now need an explicit(void *)cast at the call site to avoid a "discards const qualifier" warning.Three warnings remained after the above (all pre-existing, just never previously reached because the build died earlier):
pgtclId.c: getresid() and PgGetConnByResultId() take a
const char *handle string, then temporarily NUL the last '.'in place (and restore it) to split the connection id from the
result id.
mark = strrchr(id, '.')assigned the resultstraight into a
char *, which some toolchains flag asdiscarding the const qualifier since
idis const. Thetemporary in-place mutation itself is unchanged (it's the
existing, intentional behaviour); the cast is now explicit:
mark = (char *) strrchr(id, '.');pgtclCmds.c: Pg_select() declared
int tuplesProcessed = 0;and incremented it once per row, but never read it anywhere.
The
-count varfeature that this looks like it was meant tofeed is actually driven by a separate
numTuples/tuplesVarObjpair a few lines away, so
tuplesProcessedwas simply deadcode left over from an earlier version. It has been removed
(declaration + increment); this does not change any observable
behaviour, since its value was never used.
None of this changes pgtcl's behaviour on any currently supported Tcl version -- it only replaces removed/renamed compatibility spellings with their modern equivalents, and drops one genuinely-unused variable.
Hacked by Claude, tested by me.
Per: pgdg-packaging/pgdg-rpms#213