Fix build warnings against PG 16+ - #526
Open
devrimgunduz wants to merge 1 commit into
Open
devrimgunduz wants to merge 1 commit into
devrimgunduz wants to merge 1 commit into
Conversation
PG_ENSURE_ERROR_CLEANUP()/PG_END_ENSURE_ERROR_CLEANUP(), as defined in storage/ipc.h, expand to a plain PG_TRY()/PG_CATCH()/PG_END_TRY() with no suffix argument. Nesting two such blocks in the same function (as happens below, in pglogical_sync_subscription()) makes the inner block's PG_TRY()-generated locals shadow the outer block's, which trips -Wshadow=compatible-local on modern gcc. This is purely cosmetic: each PG_TRY() only ever touches its own copy of the saved exception/context stack, so the shadowing has no runtime effect. As of PG 16, core's PG_TRY()/PG_CATCH()/PG_END_TRY() accept an optional suffix argument specifically to let nested blocks use distinct variable names and silence this warning -- but PG_ENSURE_ERROR_CLEANUP() itself doesn't forward that suffix through. On PG 16+ we define local wrappers that do. On PG 14/15, PG_TRY() and friends take no arguments at all, so there's no way to avoid the warning short of restructuring the code (e.g. moving the inner block into a separate function); we just fall back to the plain core macros there and accept the harmless warning. These wrappers can be removed if/when upstream teaches PG_ENSURE_ERROR_CLEANUP() to forward a suffix through itself on all still-supported branches. Hacked by Claude, tested by me.
Contributor
|
I submitted a patch to -hackers. https://www.postgresql.org/message-id/a690b47b-1859-4598-8541-286e48e7a724%40app.fastmail.com |
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.
PG_ENSURE_ERROR_CLEANUP()/PG_END_ENSURE_ERROR_CLEANUP(), as defined in storage/ipc.h, expand to a plain PG_TRY()/PG_CATCH()/PG_END_TRY() with no suffix argument. Nesting two such blocks in the same function (as happens below, in pglogical_sync_subscription()) makes the inner block's PG_TRY()-generated locals shadow the outer block's, which trips -Wshadow=compatible-local on modern gcc. This is purely cosmetic: each PG_TRY() only ever touches its own copy of the saved exception/context stack, so the shadowing has no runtime effect.
As of PG 16, core's PG_TRY()/PG_CATCH()/PG_END_TRY() accept an optional suffix argument specifically to let nested blocks use distinct variable names and silence this warning -- but PG_ENSURE_ERROR_CLEANUP() itself doesn't forward that suffix through. On PG 16+ we define local wrappers that do. On PG 14/15, PG_TRY() and friends take no arguments at all, so there's no way to avoid the warning short of restructuring the code (e.g. moving the inner block into a separate function); we just fall back to the plain core macros there and accept the harmless warning.
These wrappers can be removed if/when upstream teaches PG_ENSURE_ERROR_CLEANUP() to forward a suffix through itself on all still-supported branches.
Hacked by Claude, tested by me.