rollbear / trompeloeil

Header only C++14 mocking framework
Boost Software License 1.0
811 stars 86 forks source link

Support building with Clang on Windows #258

Open laudrup opened 3 years ago

codecov[bot] commented 3 years ago

Codecov Report

Merging #258 (3683b54) into main (7cecfea) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #258   +/-   ##
=======================================
  Coverage   75.68%   75.68%           
=======================================
  Files           1        1           
  Lines         872      872           
=======================================
  Hits          660      660           
  Misses        212      212           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7cecfea...3683b54. Read the comment docs.

laudrup commented 3 years ago

Clang on Windows is now fairly well integrated into the standard Visual Studio installation including the build image used by Github actions.

This is obviously work in progress.

Seems like you're interested in supporting many different compilers/toolchains so I thought you might be interested.

I was just wondering if you're interested in me going any further with this before I look any more into this?

Thanks.

rollbear commented 3 years ago

I am indeed interested in this. Thank you! Extra thanks for getting a CI build in place, it would quickly rot otherwise. I note that it failed, though, which should be looked into. I assume it would be good to build with several different C++ standards.

Never mind the failures for old gcc/clang compilers. These are on me for not properly updating the CI environment after old Ubuntu images were retired.

laudrup commented 3 years ago

I am indeed interested in this. Thank you! Extra thanks for getting a CI build in place, it would quickly rot otherwise.

Nice to hear. It wouldn't make much sense without a CI build, so that seemed like an obvious thing to do.

I note that it failed, though, which should be looked into.

I know. Seems like there's a test to ensure than an expectation fails in the expected way. The expectation does fail, but the output doesn't match the regex in the test. Seems a bit weird, but I haven't looked into it in detail at all. Wanted to know if this PR was relevant in the first place.

I assume it would be good to build with several different C++ standards.

Sure. Again, I wanted to ensure this was interesting for you at all before going any further.

Slightly unrelated, but are you really sure it's possible to require C++11 with MSVC? As far as I know, supporting standard flags was only something MSVC supported beginning with C++14 which was around the time when MS started to seem to care about standards. I'm fairly certain /std::c++11 isn't supported by MSVC and might fall back to /std::c++-latest or something.

Never mind the failures for old gcc/clang compilers. These are on me for not properly updating the CI environment after old Ubuntu images were retired.

Thanks. I'll ignore that :-)

laudrup commented 3 years ago

@rollbear

The failing test is "C++11: unmatched call with mismatching requirements is reported". Same for C++14 specific tests.

For some reason, the error reported isn't the one expected. Instead of one error report two are generated and additionally the regex doesn't match any of those.

The reason for that is, that the code generating the report doesn't write that it tried to match both REQUIRE_CALL expectations, but only the first one and then generates a second error for the missing call to the second expectation.

That code causing the test to fail seems to be the report_mismatch function which for some reason stops the iteration after the first call to m.report_mismatch when iterating the matcher_list although the container actually contains two elements as expected.

That's fairly bizarre as I cannot see any platform/compiler specific code being used for creating the error message at all.

If you have any clue on why that might happen I would be happy to get some pointers, otherwise this is mostly just to let you know that I haven't given up on this, I just have limited time to look into it.

Also please let me know if my explanation doesn't make any sense :-)

Thanks.

laudrup commented 3 years ago

I've looked more into this and it is indeed very weird. When this is called the iteration stops.

That happens not only for the failing test, but for all tests but only when using Clang on Windows.

I don't expect this to be an issue with the trompeloeil code, but potentially a bug in the compiler. I cannot really see what else could be the cause of this.

I will try to boil the code down to a minimal reproducable example, that should hopefully provide some insight, but it will require a bit of work so I cannot promise when I'll get that done and if it is indeed an issue with Clang I'll report an issue there.

rollbear commented 3 years ago

OK, good to learn of your progress. Sorry for being a bit inattentive right now, I'm in full panic mode about doing a presentation at the NDC TechTown conference next week. I'll return to this once that is over.

laudrup commented 3 years ago

@rollbear

No problem at all. Hope you'll have a chance to give it a quick look once the panic has settled.

Good luck with the conference.

rollbear commented 2 years ago

Looking at the build, I see some extraordinarily unexpected warnings. It should build clean from warnings.

         In file included from D:\a\trompeloeil\trompeloeil\test\compiling_tests.cpp:17:
     1>D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(1318,13): warning : implicit conversion between pointer-to-function and pointer-to-object is a Microsoft extension [-Wmicrosoft-cast] [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(1423,20): message : in instantiation of member function 'trompeloeil::streamer<int (*)(int), true, false>::print' requested here [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(1451,19): message : in instantiation of member function 'trompeloeil::printer<int (*)(int)>::print' requested here [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(3232,20): message : in instantiation of function template specialization 'trompeloeil::print<int (*)(int)>' requested here [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(3243,55): message : in instantiation of function template specialization 'trompeloeil::missed_value<int (*)(int)>' requested here [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(3252,5): message : in instantiation of function template specialization 'trompeloeil::stream_params<0, int (*&)(int)>' requested here [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(3388,5): message : in instantiation of function template specialization 'trompeloeil::stream_params<int (*&)(int)>' requested here [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(4347,7): message : in instantiation of function template specialization 'trompeloeil::report_mismatch<void (int (*)(int))>' requested here [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\test/compiling_tests.hpp(361,3): message : in instantiation of function template specialization 'trompeloeil::mock_func<false, void (int (*)(int)), int (*&)(int)>' requested here [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(5223,35): message : expanded from macro 'MAKE_MOCK1' [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(4513,3): message : expanded from macro 'TROMPELOEIL_MAKE_MOCK1' [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]
       D:\a\trompeloeil\trompeloeil\include\trompeloeil.hpp(4725,27): message : expanded from macro 'TROMPELOEIL_MAKE_MOCK_' [D:\a\trompeloeil\trompeloeil\build\self_test.vcxproj]

The print/streamer functions should be instantiated with the type of each parameter. There is no parameter that is a pointer to function. The function type matches that of the mocked function in the failing test. I have no idea why it becomes so wrong for this particular test, but it seems to go wrong already when expanding the MAKE_MOCK1 macro. I'm very intrigued by mock_func<false, void (int (*)(int)), int (*&)(int)>', because that doesn't match how MAKE_MOCK1 expands.

The CI build should really be with -Werror, and very strict warnings enabled.

laudrup commented 2 years ago

Looking at the build, I see some extraordinarily unexpected warnings. It should build clean from warnings.

Agreed and I am aware of that. I have looked a bit into what the compiler warned about:

warning : implicit conversion between pointer-to-function and pointer-to-object is a Microsoft extension [-Wmicrosoft-cast]

And as far as I can tell, it's harmless as it simply means that Clang will do the same as MSVC. I'm not 100% sure though, but it didn't seem related to the actual failure (I tried commenting out the code that triggered the warning) so I decided to ignore it for now.

The print/streamer functions should be instantiated with the type of each parameter. There is no parameter that is a pointer to function. The function type matches that of the mocked function in the failing test. I have no idea why it becomes so wrong for this particular test, but it seems to go wrong already when expanding the MAKE_MOCK1 macro. I'm very intrigued by mock_func<false, void (int (*)(int)), int (*&)(int)>', because that doesn't match how MAKE_MOCK1 expands.

That's interesting. Seems like that's what I should be looking into. I guess it could very well be that the test that actually fails later on is caused by this. Thanks a lot.

The CI build should really be with -Werror, and very strict warnings enabled.

Agreed 100%. That's what I always do myself.

I'll look more into this when I have the time. Thanks for giving me some input to work on.

aminya commented 2 years ago

setup-cpp supports installing and setting up llvm for Windows. We can add this in #266