-
Notifications
You must be signed in to change notification settings - Fork 179
Feature msvc clang #141
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
Feature msvc clang #141
Conversation
…it quoted dereference in STREQUAL comparison
Don't like this. I am using an old version of Ubuntu that is LTS and that
is using CMake 2.8.12.2.
GR
…Sent from my Nexus 6P
On 15 Dec 2016 9:04 am, "Gabriel Hare" ***@***.***> wrote:
UnitTest++ supports clang as a compiler for Visual Studio via native
extension
For information on enabling & using the extension see:
https://blogs.msdn.microsoft.com/vcblog/2015/12/04/clang-
with-microsoft-codegen-in-vs-2015-update-1/
(Since that post, an update offers v140_clang_c2)
And, for instruction for CMake see:
https://github.com/boostorg/hana/wiki/Setting-up-Clang-on-Windows
However, the CMakeLists.txt for UnitTest++ uses the MSVC variable to test
for the compiler type, which will be incorrect when clang is used.
This PR uses a test against the compiler name instead. It has been
verified with:
- cmake 3.7.0 (Windows)
- clang 3.8.0 (v140_clang_c2 Visual Studio 2015)
- msvc 19.0.24215.1 (Visual Studio 2015)
- cmake 3.5.2 (Ubuntu)
- GCC 6.2.0 (Ubuntu)
- clang 3.8.1 (Ubuntu)
- CMake 3.4.3 (Apple)
- AppleClang 8.0.0 generated for XCode
- AppleClang 8.0.0 generated for command line make
NOTE: It is also possible to attempt to install and use LLVM & Clang, but
the compiler flags are a mess
http://releases.llvm.org/download.html
http://clang.llvm.org/docs/MSVCCompatibility.html
------------------------------
You can view, comment on, or merge this pull request online at:
#141
Commit Summary
- Demonstrate that compiler is identified as MSVC when VS2014 Clang
extension is used
- Increase CMake version to resolve developer warnings regarding
implicit quoted dereference in STREQUAL comparison
- Identify Clang compiler when used as an MSVC extension
File Changes
- *M* CMakeLists.txt
<https://github.com/unittest-cpp/unittest-cpp/pull/141/files#diff-0>
(14)
Patch Links:
- https://github.com/unittest-cpp/unittest-cpp/pull/141.patch
- https://github.com/unittest-cpp/unittest-cpp/pull/141.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#141>, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/AEy9nOVz8WlM0i1egoBF_NStEuikz2_Tks5rIQKFgaJpZM4LN3Iy>
.
|
@grahamreeds Makes sense, and I see the error in Travis - I'll fix & update. |
…g implicit quoted dereference in STREQUAL comparison" This reverts commit 8f7a263.
NOTE: Unlike STREQUAL this comparison is supported pre and post CMake v3.1.0
@grahamreeds update passes tests now - I hope you'll support this PR |
Your changes made me notice the portion of the CMake message that reads "Please use a different C++ compiler." That message is misleading; compilers that don't support C++11 still work with UnitTest++. As such I don't consider this a warning; please change the new and existing message to STATUS level, and while you're at it I'd be pleased if you removed that sentence. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See prior comment.
Thank you for taking a look at this. (Also, please note that this PR removes the building of TestUnitTest++ from "all" target.) |
Why do you remove building of the unit tests from the all target?
GR
…Sent from my Nexus 6P
On 20 Jan 2017 3:47 am, "Gabriel Hare" ***@***.***> wrote:
Thank you for taking a look at this.
Your requested changes appear to elevate the message level from STATUS to
WARNING and do not remove the statement that the a different compiler
should be used.
I'm happy to make your requested changes (as I understand them) or accept
the changes - please clarify what your would prefer.
(Also, please note that this PR removes the building of TestUnitTest++
from "all" target.)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#141 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEy9nNftTHFpt0iTrDznKCq_XCtLzmOCks5rUBISgaJpZM4LN3Iy>
.
|
Actually, that's not in this PR - I can open another with that (and CI changes). Regarding why: including tests in all makes development simpler, but makes rebuilding a project that uses UnitTest++ take longer. |
But why do you think it is beneficial to remove building of unittests from
the all target? You are supposed to run unittests after every build...
…Sent from my Nexus 6P
On 20 Jan 2017 22:15, "Gabriel Hare" ***@***.***> wrote:
Actually, that's not in this PR - I can open another with that (and CI
changes).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#141 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEy9nOlx6Wnl6VhgX8wZy4-E--koBS6Pks5rURXvgaJpZM4LN3Iy>
.
|
In my case I'm using UnitTest++ in my project via CMake's ExternalProject_Add so for each complete rebuild of the project I'm also rebuilding UnitTest++, and would be rebuilding TestUnitTest++ if it were not excluded. I'm guessing that I'm not unusual in building UnitTest++ when building a project. |
You can already skip building and running TestUnitTest++ by setting the CMake build option |
@pjohnmeyer thank you for the cmake flag hint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requested changes made in commit updating this PR
@pjohnmeyer do the changes made in 0b15459 work for you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These LGTM now; I resolved merge conflicts and will wait for tests to pass.
Just got the notification - thank you :-) |
UnitTest++ supports clang as a compiler for Visual Studio via native extension
For information on enabling & using the extension see:
https://blogs.msdn.microsoft.com/vcblog/2015/12/04/clang-with-microsoft-codegen-in-vs-2015-update-1/
(Since that post, an update offers v140_clang_c2)
And, for instruction for CMake see:
https://github.com/boostorg/hana/wiki/Setting-up-Clang-on-Windows
However, the CMakeLists.txt for UnitTest++ uses the MSVC variable to test for the compiler type, which will be incorrect when clang is used.
This PR uses a test against the compiler name instead. It has been verified with:
NOTE: It is also possible to attempt to install and use LLVM & Clang, but the compiler flags are a mess
http://releases.llvm.org/download.html
http://clang.llvm.org/docs/MSVCCompatibility.html