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.

Sign up for GitHub

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

Jump to bottom

VS2010 x86 doesn't accept LLU suffix on numbers #3647

Closed
ip7z opened this issue May 14, 2023 · 1 comment · Fixed by #3929
Closed

VS2010 x86 doesn't accept LLU suffix on numbers #3647

ip7z opened this issue May 14, 2023 · 1 comment · Fixed by #3929
Assignees
@Cyan4973
Labels
Portability

Comments

@ip7z
Copy link

ip7z commented May 14, 2023

VS2010 x86 doesn't accept LLU suffix in numbers:
C2059: syntax error : 'bad suffix on number'.
zstd.h file:

#define ZSTD_MAX_INPUT_SIZE ((sizeof(size_t)==8) ? 0xFF00FF00FF00FF00LLU : 0xFF00FF00U)

ULL suffix probably fixes such issues.
@Cyan4973
Copy link
Contributor

Cyan4973 commented May 16, 2023 •
edited
Loading

Note: there are Visual Studio tests run in CI, and so far, none of them seems to suffer from LLU vs ULL. Btw, both suffixes are supposed to be valid according to C99 and C11 standards.
So I guess this outcome may depend on the exact version number of Visual Studio, and it's unfortunate that we can't test the failing versions with Github Actions, presumably because they are older than support limit. This situation would force us to deploy a "blind" fix, without observing the success (or the drawbacks).

@Cyan4973 Cyan4973 self-assigned this May 16, 2023
@Cyan4973 Cyan4973 mentioned this issue May 31, 2023
Merged
Cyan4973 added a commit that referenced this issue Mar 4, 2024
@Cyan4973
fix LLU->ULL
2abe8d6 
LLU is a correct prefix according to C99 & C11 standards (but not C90).
However, older versions of Visual Studio do not work with it.
Replace by ULL, which doesn't have this issue.

Fixes #3647
@Cyan4973 Cyan4973 mentioned this issue Mar 4, 2024
Merged
hswong3i pushed a commit to alvistack/facebook-zstd that referenced this issue Mar 27, 2024
@Cyan4973 @hswong3i
fix LLU->ULL
bce72d9 
LLU is a correct prefix according to C99 & C11 standards (but not C90).
However, older versions of Visual Studio do not work with it.
Replace by ULL, which doesn't have this issue.

Fixes facebook#3647
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Cyan4973 Cyan4973

Labels
Portability
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix LLU->ULL facebook/zstd
2 participants
@Cyan4973 @ip7z