Skip to content

Attempt to fix VS build by defining noexcept#227

Closed
dmah42 wants to merge 36 commits intomasterfrom
fixvscheck
Closed

Attempt to fix VS build by defining noexcept#227
dmah42 wants to merge 36 commits intomasterfrom
fixvscheck

Conversation

@dmah42
Copy link
Member

@dmah42 dmah42 commented May 27, 2016

No description provided.

@dmah42
Copy link
Member Author

dmah42 commented May 27, 2016

@BillyONeal any chance you can try this to see if it works for VS? I have no access to a VS machine.

dmah42 and others added 4 commits May 27, 2016 15:28
* Move ComputeStats call out of the reporters

* Cleanup adjusted time calculations in reporters

* Move ComputeBigO call out of reporters

* Remove ReportComplexity interface using ReportRuns instead

* Factor out reporting of basic context information

* Attempt to fix GCC 4.6 build errors

* Move ComputeStats to complexity.cc
@coveralls
Copy link

Coverage Status

Coverage remained the same at 87.808% when pulling e515a66 on fixvscheck into 238e558 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 88.294% when pulling f2a1e44 on fixvscheck into 238e558 on master.

@BillyONeal
Copy link
Contributor

I don't have super easy access to a VS2013 machine, and VS2015 supports conditional noexcept (so it works unmodified with the code in question), so it'll take a while for me to try. The build is already broken so if you don't want to wait for me to setup a VS2013 machine you may as well just merge it :)

# define BENCHMARK_UNUSED
# define BENCHMARK_ALWAYS_INLINE __forceinline
# define BENCHMARK_NOEXCEPT
# define BENCHMARK_NOEXCEPT(x)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should test _MSC_VER so that it uses conditional noexcept in 2015 or noexcept(false) cases will go to terminate() there.

@dmah42
Copy link
Member Author

dmah42 commented May 27, 2016

@BillyONeal ok. i know i fixed something, at least. but i think there's more to do with the noexcept macro and cmake.

@BillyONeal
Copy link
Contributor

@dominichamon You may need separate macros; I know in the STL we have _NOEXCEPT for unconditional noexcept and _NOEXCEPT_OP(x) for conditional noexcept

@dmah42
Copy link
Member Author

dmah42 commented May 27, 2016

slowest. edit compile loop. ever.

@BillyONeal
Copy link
Contributor

Yeah; it's a real shame that AppVeyor is so much slower here than travis-ci :(

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 88.294% when pulling 0948b38 on fixvscheck into 238e558 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 88.294% when pulling 0948b38 on fixvscheck into 238e558 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 88.294% when pulling f572d6b on fixvscheck into 238e558 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 88.294% when pulling 5b14d00 on fixvscheck into 238e558 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 88.294% when pulling f55649f on fixvscheck into 238e558 on master.

@dmah42
Copy link
Member Author

dmah42 commented May 31, 2016

i wish i could clean up the commit history here... @EricWF whenever you can, please persuade cla-bot that i'm not stealing your work :)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 88.294% when pulling 1e2fca5 on fixvscheck into 238e558 on master.

@BillyONeal
Copy link
Contributor

OK, I got a VS2013 machine to test with. How much do you guys really care about 2013? :)

FAILED: C:\PROGRA~2\MICROS~1.0\VC\bin\cl.exe   /nologo /TP -DHAVE_STD_REGEX -DHAVE_STEADY_CLOCK -D_CRT_SECURE_NO_WARNINGS -I..\include -I..\src /DWIN32 /D_WINDOWS  /GR /EHsc /W4 /D_DEBUG /MDd /Zi /Ob0 /Od /RTC1 /showIncludes /Fosrc\CMakeFiles\benchmark.dir\benchmark.cc.obj /Fdsrc\CMakeFiles\benchmark.dir\ /FS -c ..\src\benchmark.cc
C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\INCLUDE\xxatomic(229) : error C2664: 'std::_Atomic_address &std::_Atomic_address::operator =(const std::_Atomic_address &)' : cannot convert argument 1 from 'const char *' to 'void *'
        Conversion loses qualifiers
        C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\INCLUDE\xxatomic(227) : while compiling class template member function 'std::atomic<const char *>::atomic(_Ty *) throw()'
        with
        [
            _Ty=const char
        ]
        ..\src\benchmark.cc(118) : see reference to function template instantiation 'std::atomic<const char *>::atomic(_Ty *) throw()' being compiled
        with
        [
            _Ty=const char
        ]
        ..\src\benchmark.cc(118) : see reference to class template instantiation 'std::atomic<const char *>' being compiled
[14/37] Building CXX object src\CMakeFiles\benchmark.dir\colorprint.cc.obj
ninja: build stopped: subcommand failed.

@BillyONeal
Copy link
Contributor

If you want I can make a separate PR with changes to fix VS2013

@dmah42
Copy link
Member Author

dmah42 commented May 31, 2016

yeah, i was seeing that on appveyor too. haven't managed to come up with a good solution yet that's clean.

if you have the time, go for it. otherwise i'll keep hacking on this.

@BillyONeal
Copy link
Contributor

@dmah42
Copy link
Member Author

dmah42 commented May 31, 2016

@BillyONeal right, that's something close to what i had. the const_cast bothered me, and the version check (instead of BENCHMARK_OS_WINDOWS or COMPILER_MSVC) but it may be that this is what we need to do.

you can land that independently of this PR, right?

@BillyONeal
Copy link
Contributor

Yeah; I have to figure out how to fix the author on those commits first -- I had forgotten to setup my name correctly on the throwaway VS2013 VM where I made these changes :/

@EricWF
Copy link
Contributor

EricWF commented May 31, 2016

Sorry I just saw your comments today. I'm OK with all of this.

@dmah42
Copy link
Member Author

dmah42 commented May 31, 2016

@EricWF no problem. can you convince cla-bot?

@EricWF
Copy link
Contributor

EricWF commented May 31, 2016

@googlebot I'm Okay with these commits. How do I convince you?

@EricWF
Copy link
Contributor

EricWF commented May 31, 2016

@googlebot I signed it.

@dmah42 dmah42 closed this May 31, 2016
@dmah42 dmah42 deleted the fixvscheck branch May 31, 2016 20:45
@BillyONeal
Copy link
Contributor

@dominichamon Do you want me to turn that commit into a PR or are you still pursuing an alternate solution?

@dmah42
Copy link
Member Author

dmah42 commented May 31, 2016

@BillyONeal please do. i could do it but you wrote the commit already :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants