app-shells/squirrelsh: Fix building with GCC-6 #4693
Conversation
gentoo-repo-qa-bot
added
the
assigned
|
Pull Request assignment Areas affected: ebuilds app-shells/squirrelsh: @blueness |
| #include <string.h> | ||
| #include <string> | ||
|
|
||
| +#include "common.h" |
There was a problem hiding this comment.
from your report, would it be possible to fix this properly once and for all? I never use math.h in C++, use fully qualified std::min and std::max everywhere? don't see this as a requirement, it would just be a nice thing for the ecosystem, as it would make the codebase more robust, and less flaky.
There was a problem hiding this comment.
I generally try to keep my patches to a minimum to enable the code's original pre-GCC6 intent without necessarily using more proper code constructs that may have more wide-ranging consequences throughout.
In the case of app-shells/squirrelsh, there exists in ${S}/common.h:
#if !defined(min)
# define min(a, b) ((a) < (b) ? (a) : (b))
#endif
This macro name forcibly hides all overloaded declarations of std::min (such as std::min(const T& a, const T& b, Compare comp)). Specifically, the problem happens because there is a call to std::min(const T& a, const T& b, Compare comp) in /usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/include/g++-v6/bits/stl_algobase.h and that declaration has been hidden by the presence of a macro with the same name.
However, testing what would happen if I were to change that code in ${S}/common.h to:
#include <algorithm>
using std::min;
I get errors like:
hash_adler32.cpp:90:50: error: no matching function for call to 'min(SQInteger&, int)'
size_t r = fread(block, 1, min(left, BLOCK_SIZE), file);
^
because std::min places more stringent demands on its arguments (such as requiring them to be the exact same type). It would require explicit casts in more files. At that point, I'd prefer to let upstream decide how to deal with it.
There was a problem hiding this comment.
from your report, would it be possible to fix this properly once and for all?
@SoapGentoo Upstream did the work and fixed this for good.. Pushed a new patch based on https://sourceforge.net/p/squirrelsh/git/ci/13280cdf34d96216b4d10b9ea7656a42a967fc2b/
Bug: https://bugs.gentoo.org/show_bug.cgi?id=594466 Package-Manager: Portage-2.3.6, Repoman-2.3.2
|
😞 The QA check for this pull request has found the following issues: Issues inherited from Gentoo (may be modified by PR): |
Bug: https://bugs.gentoo.org/show_bug.cgi?id=594466
Package-Manager: Portage-2.3.5, Repoman-2.3.2
Upstream ticket with patch info: https://sourceforge.net/p/squirrelsh/bugs/6/