Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

silence -Wunused-function on MacOS-arm64 #382

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

divinity76
Copy link
Contributor

@divinity76 divinity76 commented Feb 3, 2024

the #endif was in the wrong place, on MacOS-arm64 causing the error /Users/runner/work/php-src/php-src/ext/hash/blake3/upstream_blake3/c/blake3_dispatch.c:112:5: error: unused function 'get_cpu_features' [-Werror,-Wunused-function]

full compiler log showcasing the issue https://github.com/php/php-src/actions/runs/7762643678/job/21173438425?pr=13194

the #endif was in the wrong place, causing the error
/Users/runner/work/php-src/php-src/ext/hash/blake3/upstream_blake3/c/blake3_dispatch.c:112:5: error: unused function 'get_cpu_features' [-Werror,-Wunused-function]

full compiler log https://github.com/php/php-src/actions/runs/7762643678/job/21173438425?pr=13194
divinity76 added a commit to divinity76/php-src that referenced this pull request Feb 4, 2024
patches to specifically address a gcc -Werror=logical-op issue explained in BLAKE3-team/BLAKE3#380
and a gcc -Wunused-function issue explained in BLAKE3-team/BLAKE3#382
and optimized upstream git checkout to only fetch the files we want.
@oconnor663
Copy link
Member

It looks like this move makes #if defined(IS_X86) on line 119 redundant. The intention was to eventually use get_cpu_features on other platforms, though we haven't done that yet. Is there any more direct way to mark the function "don't warn if this is unused"? Or is that all ugly compiler specific macros that we don't want to touch?

@divinity76
Copy link
Contributor Author

divinity76 commented Feb 4, 2024

C23 has a solution: https://en.cppreference.com/w/c/language/attributes/maybe_unused
but it's 9 years too early to require C23 compatibility.

Maybe make get_cpu_features() always static, and make a public non-static blake3_get_cpu_features() which is just a wrapper for get_cpu_features() ? that would get rid of the unused warning.

if you go that route, maybe some more thought should be put into exactly what the public function should return tho, idk

@divinity76
Copy link
Contributor Author

a possible alternative solution: #383

(it may solve the problem for ARM specifically, but probably not for other non-x86-non-arm architectures)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants