Skip to content

Comments

Fix new clang warning in library_wasm_worker.c. NFC#18936

Merged
sbc100 merged 1 commit intomainfrom
fix_llvm_warning
Mar 10, 2023
Merged

Fix new clang warning in library_wasm_worker.c. NFC#18936
sbc100 merged 1 commit intomainfrom
fix_llvm_warning

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Mar 10, 2023

Clang recently started generating a warning if we assert that a paramater marked as nonnull is not null. The code in question is:

assert(stackLowestAddress != 0);

And the warning is:

library_wasm_worker.c:32:9: warning: comparison of nonnull parameter 'stackLowestAddress' not equal to a null pointer is 'true' on first encounter [-Wtautological-pointer-compare]

Clang recently started generating a warning if we assert that a
paramater marked as `nonnull` is not null.  The code in question is:

assert(stackLowestAddress != 0);

And the warning is:

library_wasm_worker.c:32:9: warning: comparison of nonnull parameter
'stackLowestAddress' not equal to a null pointer is 'true' on first
encounter [-Wtautological-pointer-compare]
@sbc100 sbc100 requested review from dschuff and juj March 10, 2023 18:57
@sbc100
Copy link
Collaborator Author

sbc100 commented Mar 10, 2023

This unblocks the llvm roller.

@dschuff
Copy link
Member

dschuff commented Mar 10, 2023

Would it make more sense to just fix the code? I assume the compiler is just going to compile the check away to nothing anyway?
Hm, but it might still be useful in debug mode...

Copy link
Member

@dschuff dschuff left a comment

Choose a reason for hiding this comment

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

I guess we need to unblock the roller.

@sbc100
Copy link
Collaborator Author

sbc100 commented Mar 10, 2023

Would it make more sense to just fix the code? I assume the compiler is just going to compile the check away to nothing anyway? Hm, but it might still be useful in debug mode...

Yeah my first thought was to remove the assert... but actually it seems like it still useful to have maybe?

@sbc100 sbc100 merged commit 5ba1fd2 into main Mar 10, 2023
@sbc100 sbc100 deleted the fix_llvm_warning branch March 10, 2023 20:19
@dschuff
Copy link
Member

dschuff commented Mar 10, 2023

You think it's better to disable the warning with a flag like this, or a pragma in the file?

impact-maker pushed a commit to impact-maker/emscripten that referenced this pull request Mar 17, 2023
…18936)

Clang recently started generating a warning if we assert that a
paramater marked as `nonnull` is not null.  The code in question is:

assert(stackLowestAddress != 0);

And the warning is:

library_wasm_worker.c:32:9: warning: comparison of nonnull parameter
'stackLowestAddress' not equal to a null pointer is 'true' on first
encounter [-Wtautological-pointer-compare]
impact-maker pushed a commit to impact-maker/emscripten that referenced this pull request Mar 17, 2023
…18936)

Clang recently started generating a warning if we assert that a
paramater marked as `nonnull` is not null.  The code in question is:

assert(stackLowestAddress != 0);

And the warning is:

library_wasm_worker.c:32:9: warning: comparison of nonnull parameter
'stackLowestAddress' not equal to a null pointer is 'true' on first
encounter [-Wtautological-pointer-compare]
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