Update the sources so they will compile under Visual Studio #163
Conversation
|
Hi, For the hb-ot-layout-private.hh case, I can get away by casting the values used in the bitwise or with unsigned int's, such as the following, but it is clearly not an optimal solution (since, we would have to do the same for this case for GLib's gnode.h, and any enums that do bitwise or): HB_OT_LAYOUT_GLYPH_PROPS_PRESERVE = So this is the take I have for this... With blessings, thank you! |
fanc999
force-pushed
the
msvc.src
branch
2 times, most recently
from
1c0fc87 to
72d1987
Compare
|
Hello Behdad, The thing is, Visual Studio 2013 can be quite picky on the types used in lround() and scalbnf(), since we are doing C++, so one would get an ambiguous type error as the compiler insists on a type being specified, between double and float, for instance. |
fanc999
force-pushed
the
msvc.src
branch
2 times, most recently
from
f94322b to
4263e6d
Compare
|
Hi Behdad, I had your suggested changes in there, but since I rebased to make the history look less cluttered, so probably your in-line comments were taken away by this. Sorry about this-please let me know if you would rather that I just patch on top of a previous patch rather than rebasing. With blessings, thank you! |
fanc999
force-pushed
the
msvc.src
branch
2 times, most recently
from
c4de1c2 to
dd4ad1a
Compare
|
Hi, This patch will make the enum in hb-ot-layout-private.hh strong-typed on MSVC>=2012, but does not cover the scenario where we include the GLib headers (since GLib is "external" to this package), as any enums in there that use bitwise operators within them will break on MSVC>=2012, due to the same problem. Unlike g++, I don't think there is any switch in the later Visual Studio versions to disable C++-11 items (or, rather, to enable it optionally), this is how life would go in there. Perhaps, a configure check may be needed for enabling strong-typing and using the bitwise operator overloading conditionally, but this probably belonga to another PR. |
|
Everything looks good except for the last commit, which I left comments about. Thanks. |
Use the fallback implementation for lround() only on pre-2013 Visual Studio, and ensure we are clear about the types of the parameters for lround() and scalbnf(), since Visual Studio can be quite picky on ambiguous parameter types. Also, use g_ascii_strcasecmp() rather than strcasecmp() as we are already using GLib for this code and we are assured that g_ascii_strcasemp() is available. For scalbnf() on pre-2013 Visaul Studio, a fallback implementation is needed, but use another forced-included header for those compilers, which will be added later. Also use (char)27 on Visual Studio builds as '\e' is not a recognized escape sequence, which will do the same thing.
Pre-2013 MSVC does not have scalbn() and scalbnf(), which are used in the utility programs. Add fallback implementations for these, which can be used when necessary.
Visual Studio does not like declaring a enum variable within a for statement, so fix the build by declaring the enum before doing the for loop.
fanc999
force-pushed
the
msvc.src
branch
2 times, most recently
from
6515ccf to
1860a01
Compare
|
Hi, Finally a solution for the newer MSVC builds... Re-did the last patch for this. With blessings, thank you! |
Use the DEFINE_ENUM_FLAG_OPERATORS macro in winnt.h on Visual Studio,
which defines the bitwise operators for the enumerations that we want to
mark as hb_mark_as_flags_t, which will take care of the situation on newer
Visual Studio (>= 2012), where the build breaks with C2057 errors as the
underlying types of the enumerations is not clear to the compiler when we
do a bitwise op within the declaration of the enumerations themselves.
Also disable the C4200 (nonstandard extension used : zero-sized array in
struct/union) and C4800 ('type' : forcing value to bool 'true' or 'false'
(performance warning)) warnings as the C4200 is the intended scenario and
C4800 is harmless but is so far an unavoidable side effect of using
DEFINE_ENUM_FLAG_OPERATORS.
|
I still don't understand or like the casts before calling lround, etc. They make the code un-modifiable, as in, when I write new code, I'll never know when I need to put those casts. But I'm going to commit this since I don't have more time to send this back to you and review again. The rest looks good. Thanks. |
Update the sources so they will compile under Visual Studio
Update homebrew on Travis. This is required to get the current versions of Harfbuzz that are bundled, so that ctest will check against those. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-harfbuzz/163) <!-- Reviewable:end -->
Hi,
Sorry for this, was not that good at git and retried a pull request as a result.
I have made some updates to the sources in util/, and also added an implementation of scalbn() and scalbnf() in a header that can be force-included by pre-2013 Visual Studio, so that the tools can be built under Visual Studio as well.
However, it also seems that some more changes are needed... on Visual Studio 2010 and 2008, things went fine, but on 2012 and later, I am getting C2057 (expected constant expression) errors for:
hb-ot-shape-normalize.cc
hb-ot-shape.cc
hb-ot-shape-fallback.cc
When they include hb-ot-layout-private.hh, the error was at line 67 (in the declaration of enum hb_ot_layout_glyph_props_flags_t)
and the C2057 error for hb-glib.cc when it included gnode.h (at line 43 of gnode.h, when the enum GTraverseFlags was declared) from GLib...
There were bitwise or's in the declaration of the enums, so my guess is that the newer Visual Studio compilers are more picky about the overloading of operators, type-wise, or something conflicted in there, as done in hb-private.hh (circa lines 899 to 900)
With blessings, thank you!