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

PCRE2 requires C99 support #1041

Closed
tobil4sk opened this issue Apr 10, 2023 · 9 comments · Fixed by #1042
Closed

PCRE2 requires C99 support #1041

tobil4sk opened this issue Apr 10, 2023 · 9 comments · Fixed by #1042

Comments

@tobil4sk
Copy link
Member

tobil4sk commented Apr 10, 2023

This is from Lime's github actions android builds:

hxcpp/4,3,2/project/thirdparty/pcre2-10.42/src/pcre2_script_run.c:109:1: error: 'for' loop initial declarations are only allowed in C99 or C11 mode
 for (int i = 0; i < FULL_MAPSIZE; i++) require_map[i] = 0;
 ^
note: use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code

Seems like the new version of pcre from #1037 doesn't compile with older c standards.

It looks like this is why this commit was necessary? 27210f2 @hughsando

Maybe we can set the flag only for the pcre sources? Otherwise, it requires adding the c99/c11 flag to the remaining toolchains.

@player-03
Copy link
Contributor

player-03 commented Apr 10, 2023

I tried to build on my own machine. When I used android-toolchain-gcc.xml I got this error, but android-toolchain-clang.xml succeeded.

Edit: actually I got the same error in a different file. Didn't even make it as far as compiling pcre.

@hughsando
Copy link
Member

-std=c99 on the toolchain breaks some other c code (SDL) . I think -std=gnu99 might be best for all gcc based toolchains (android - others too?) without breaking anything. Or -std=c99 on the PCRE flags only. Not sure if windows will like this or not.

@player-03
Copy link
Contributor

Adding the flag to the toolchain produces a huge number of "valid for C/ObjC but not for C++" warnings when compiling other libraries. Let's try the PCRE-specific approach.

@tobil4sk
Copy link
Member Author

Not sure if windows will like this or not

MSVC doesn't recognise the -std=c99 flag, however, we don't need anything there anyway because the default MSVC C standard seems to support the for loop declaration.

I used the MSVC_VER to limit the usage to prevent it from being passed to msvc: #1042.

Could you see if the android build works with this patch in #1042? @player-03

Adding the flag to the toolchain produces a huge number of "valid for C/ObjC but not for C++"

If it's added to the toolchain, it should be added as a cflag so that it's only added to c source files. Perhaps you added it as a cppflag by mistake?

@player-03
Copy link
Contributor

Could you see if the android build works with this patch

Testing now.

If it's added to the toolchain, it should be added as a cflag so that it's only added to c source files. Perhaps you added it as a cppflag by mistake?

Actually I added it as a flag. But I'll keep in mind that cflag is an option, for future reference.

@Galactic00
Copy link

Having this same issue did you ever figure it out?

@tobil4sk
Copy link
Member Author

Having this same issue did you ever figure it out?

This is fixed in the git version of hxcpp but there hasn't been a new release to haxelib yet. You can install from git using:

haxelib git hxcpp https://github.com/HaxeFoundation/hxcpp

@Galactic00
Copy link

thanks but I already tried that. I specified more at openfl/lime#1682

@tobil4sk
Copy link
Member Author

It looks like this is your error: #1064

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 a pull request may close this issue.

4 participants