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

Fix GCC build issue with Fedora (linux) #4496

Merged
merged 1 commit into from Jan 13, 2017
Merged

Conversation

Mystro256
Copy link
Contributor

@Mystro256 Mystro256 commented Dec 6, 2016

Building dolphin with Fedora 26 (in development) seems to cause the following error:

/builddir/build/BUILD/dolphin-5.0/Source/Core/VideoBackends/OGL/RasterFont.cpp:18:18: error: expected unqualified-id before numeric constant
 static const int CHAR_WIDTH = 8;

I assume this is because CHAR_WIDTH already exists as a "#define" somewhere.
Making it lowercase fixes the issue. I also changed the rest of the const's to lowercase for consistency.

EDIT: I've reworked it so CHAR_ is replaced by CHARACTER_ so style can be preserved. see below.

@JosJuice
Copy link
Member

Our convention for Dolphin is to use uppercase for constants even if they aren't defines. Could you change the names to something else? Maybe CHARACTER instead of CHAR?

@BhaaLseN
Copy link
Member

And I believe those can be made constexpr too?

@Mystro256
Copy link
Contributor Author

@JosJuice Sounds good, I'll rework it to use "CHARACTER"

@BhaaLseN What version of C++ are you targeting? 11? Unfortunately work mostly with C++03 and before, so I'm a little unfamiliar with the new stuff added in 11.

@Mystro256
Copy link
Contributor Author

Done, see new commit

@JosJuice
Copy link
Member

The lint bot (running clang-format) is failing, so you'll need to format the lines the way it shows. (The max length for a line is 100 characters.)

Fixes build issue with GCC/CPP 6.2.1 build issues on Fedora 26
@Mystro256
Copy link
Contributor Author

Gotcha, I've made the style fixes and squashed the commits

@Parlane Parlane merged commit ad84b90 into dolphin-emu:master Jan 13, 2017
@Parlane
Copy link
Member

Parlane commented Jan 13, 2017

Thanks @Mystro256

@HarleyGaniere
Copy link

Thank you @Mystro256, as both a Fedora and Dolphin user this means a lot.

@Mystro256
Copy link
Contributor Author

No problem, I maintain the official fedora dolphin package for a reason:
https://admin.fedoraproject.org/pkgdb/package/rpms/dolphin-emu/

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