Open
Conversation
Owner
|
I have broken down the big branching function by moving some of the commandline options to the modules they belong to. Hopefully the parts are OK. |
|
Somehow I completely missed this! I'll have to check this in a few hours... |
Quuxplusone
force-pushed
the
appveyor-ci
branch
2 times, most recently
from
85d1d21 to
8c1aca3
Compare
Contributor
Author
|
Update: MSVC seems to be able to compile hyper.cpp without hitting that internal limit, which is cool. But now MSVC's linker is unhappy for a different reason. I'll look into this. In the meantime, I think it would be safe to cherry-pick efc9e81 if you wanted. |
Contributor
Author
|
Confirmed — |
|
MSYS2/MinGW-W64 is confirmed to work with the new Makefile.simple |
Quuxplusone
force-pushed
the
appveyor-ci
branch
5 times, most recently
from
d400c73 to
60bca9b
Compare
Quuxplusone
force-pushed
the
appveyor-ci
branch
from
60bca9b to
bb94b2b
Compare
Contributor
Author
|
Rebased. |
Quuxplusone
force-pushed
the
appveyor-ci
branch
from
bb94b2b to
4aae7f3
Compare
Lambdas to be stored in `function<void()>` should not return `bool`. Two uses of `std::function` could be just `function`, like everywhere else in the codebase.
…e time.
Before:
time c++ -O2 -DMAC -I/usr/local/include -std=c++11 -march=native
-W -Wall -Wextra -Werror -pedantic -Wno-format-pedantic
-Wno-missing-field-initializers -Wno-unused-parameter
-DCAP_GLEW=0 -DCAP_PNG=0 -c hyper.cpp -o hyper.o
real 2m22.508s
user 2m20.625s
sys 0m1.648s
After:
time c++ -O2 -DMAC -I/usr/local/include -std=c++11 -march=native
-W -Wall -Wextra -Werror -pedantic -Wno-format-pedantic
-Wno-missing-field-initializers -Wno-unused-parameter
-DCAP_GLEW=0 -DCAP_PNG=0 -c hyper.cpp -o hyper.o
real 1m30.515s
user 1m29.793s
sys 0m0.689s
Comparing object file size:
-rw-r--r-- 1 ajo staff 8215036 Jan 5 20:46 old-hyper.o
-rw-r--r-- 1 ajo staff 7538072 Jan 5 20:47 new-hyper.o
Comparing number of symbols:
nm old-hyper.o | wc -l => 12590
nm new-hyper.o | wc -l => 9742
No appreciable difference in link time; the linker takes less than
half a second in either case.
Suppress some spammy warnings on MSVC.
/wd4068 "unknown pragma"
/wd4244 "possible loss of data"
The `setlocal` at the top of appveyor.bat is the appropriate fix for
"Input line too long", according to
https://stackoverflow.com/a/19929778/1424877
https://stackoverflow.com/questions/16821784/input-line-is-too-long-error-in-bat-file#comment63959177_19929778
It keeps the global PATH from getting longer every time appveyor.yml
reinvokes appveyor.bat. Besides, having appveyor.bat change the global PATH
is almost certainly a terrible idea!
C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.15.26726\include\utility(241):
warning C4267: 'initializing': conversion from 'size_t' to '_Ty1', possible loss of data
with [_Ty1=int]
MSVC cannot handle `extern` declarations inside a member function, when that member function is itself inside a namespace: it "forgets" the namespace part when generating the symbol, which leads in this case to an "unresolved symbol" error at link time. Reduced example: https://godbolt.org/z/YSQ--9
Quuxplusone
force-pushed
the
appveyor-ci
branch
from
4aae7f3 to
d79d261
Compare
MSVC requires `-D_USE_MATH_DEFINES=1` in order to be standards-conforming. https://stackoverflow.com/questions/6563810/m-pi-works-with-math-h-but-not-with-cmath-in-visual-studio util.cpp(197): error C2065: 'M_PI': undeclared identifier MSVC warns on implicit double-to-float conversions: screenshot.cpp(825): warning C4305: 'argument': truncation from 'double' to 'float' screenshot.cpp(827): warning C4305: 'argument': truncation from 'double' to 'float' screenshot.cpp(829): warning C4305: 'argument': truncation from 'double' to 'float'
Quuxplusone
force-pushed
the
appveyor-ci
branch
from
25e6cc8 to
a05eff5
Compare
Contributor
Author
|
TravisCI supports Windows now! But I haven't figured out how to build HyperRogue on it yet. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I'm not sure of the cost/benefit here, but there's no harm in posting a pull request. :)
Benefit: Appveyor will test the MinGW and MSVC builds!
Cost: 120 lines of Appveyor-specific .yml and .bat files. Also, you'll have to give GitHub permissions to Appveyor — and it asks for more permissions than Travis, in suspiciously broken English. 😕
I'd also want to hear from @Danfun64 that I'm not messing up his MinGW build with this change to Makefile.simple.
-DMSVC_COMPILER_LIMITSis only because MSVC is stupid (and non-conforming). I haven't thought of a better easy way to fix that fatal error. I think better but complicated ways would be to rewrite that function as some kind of lookup into a big table of function pointers (expressed as lambdas), or break it down into subfunctions at the points currently indicated by comments (// cheats,// mode changes:,// informational:, etc).