Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support building with Clang on Windows #258

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

laudrup
Copy link
Contributor

@laudrup laudrup commented Oct 3, 2021

No description provided.

@codecov
Copy link

codecov bot commented Oct 3, 2021

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
Copy link
Contributor Author

laudrup commented Oct 3, 2021

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
Copy link
Owner

rollbear commented Oct 4, 2021

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
Copy link
Contributor Author

laudrup commented Oct 4, 2021

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
Copy link
Contributor Author

laudrup commented Oct 11, 2021

@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
Copy link
Contributor Author

laudrup commented Oct 15, 2021

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
Copy link
Owner

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
Copy link
Contributor Author

laudrup commented Oct 19, 2021

@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
Copy link
Owner

rollbear commented Nov 5, 2021

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
Copy link
Contributor Author

laudrup commented Nov 5, 2021

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
Copy link

aminya commented Dec 7, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants