Skip to content

Replace "alternative" boolean operator in conditional compilation directive #6949

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

Merged
merged 1 commit into from
Apr 27, 2024

Conversation

mgroeber9110
Copy link
Contributor

While looking over the warnings that MS VisualC produces, I noticed this oddity that looks like a typo in an #if directive, or at least something that is highly non-portable.

I don't have the LLVM compiler to test, but this just doesn't look right...

@slaren
Copy link
Member

slaren commented Apr 27, 2024

@mgroeber9110
Copy link
Contributor Author

Thanks a lot for the reference! I was not aware of this part of the standard, and I think I have never seen that form being used anywhere before.

I noticed that it produces >100 warnings under Visual Studio 2022 (complaining about unexpected tokens at the end of a preprocessor directive).

Some more searching brought up this: https://stackoverflow.com/questions/24414124/why-does-vs-not-define-the-alternative-tokens-for-logical-operators - apparently Visual Studio supports this, but only by either including a header <iso646.h> or by adding the flag /permissive- to enforce greater standards compliance (in recent versions of VS).

As this is the only place in the project where this is used, my proposal would still be to change it to use the more common || form that is used everywhere else in the code, even if just for consistency and to make build output on Windows a bit less noisy.

@mgroeber9110 mgroeber9110 changed the title Fix incorrect boolean operator in conditional compilation directive Replace "alternative" boolean operator in conditional compilation directive Apr 27, 2024
@slaren slaren merged commit 4dba7e8 into ggml-org:master Apr 27, 2024
55 of 56 checks passed
@mgroeber9110 mgroeber9110 deleted the vc-warning-cleanup branch April 27, 2024 19:08
nopperl pushed a commit to nopperl/llama.cpp that referenced this pull request May 5, 2024
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