Skip to content

Fix -Wunused-but-set-global warnings - #25245

Draft
arnaud-lb wants to merge 2 commits into
php:masterfrom
arnaud-lb:unused-but-set-global
Draft

arnaud-lb wants to merge 2 commits into
php:masterfrom
arnaud-lb:unused-but-set-global

Conversation

@arnaud-lb

@arnaud-lb arnaud-lb commented Nov 6, 2026 •

Copy link
Copy Markdown
Member

Clang 23 complains about static globals that are set but never read

@Girgias Girgias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was slightly confused why the php_token_ce is unused, the reason is that PHP_METHOD(PhpToken, tokenize) gets the token class via: token_class = zend_get_called_scope(execute_data); which may seems odd at first, but it's to allow the class to be extended and creates instances of the subclass. (Which probably should have a comment to explain it).

The zend_test are quite obviously correct.

For the user_filter_class_entry it currently makes sense as it is only ever used as an "implicit" requirement to be extended, but I've thought that maybe we should require that registering a userland filter extends from it. But then the API is very clunky (defining onCreate/onClose methods rather than using normal constructor/destructor) so maybe creating a new interface would just be better.

Comment thread main/rfc1867.c Outdated
static php_rfc1867_encoding_translation_t php_rfc1867_encoding_translation = dummy_encoding_translation;
static php_rfc1867_get_detect_order_t php_rfc1867_get_detect_order = NULL;
static php_rfc1867_set_input_encoding_t php_rfc1867_set_input_encoding = NULL;
static php_rfc1867_set_input_encoding_t ZEND_ATTRIBUTE_UNUSED php_rfc1867_set_input_encoding = NULL;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must the unused attribute be placed here? Can it not be at the front?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants