Skip to content
New issue

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

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

Already on GitHub? # to your account

Backport LLVM's r341717 "Fix flags used to compile benchmark library with clang-cl" #673

Merged
merged 1 commit into from
Sep 10, 2018

Conversation

LebedevRI
Copy link
Collaborator

MSVC is true for clang-cl, but "${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" is false, so we would enable -Wall, which means -Weverything
with clang-cl, and we get tons of undesired warnings.

Use the simpler condition to fix things.

Patch by: Reid Kleckner @rnk
https://reviews.llvm.org/rL341717
llvm-mirror/llvm@a8ea812

…with clang-cl"

`MSVC` is true for clang-cl, but `"${CMAKE_CXX_COMPILER_ID}" STREQUAL
"MSVC"` is false, so we would enable -Wall, which means -Weverything
with clang-cl, and we get tons of undesired warnings.

Use the simpler condition to fix things.

Patch by: Reid Kleckner @rnk
@EricWF
Copy link
Contributor

EricWF commented Sep 10, 2018

LGTM. Though I think we should get @rnk's sign off explicitly if we're literally just going to steal the commit.

@rnk
Copy link
Contributor

rnk commented Sep 10, 2018

Looks good, thanks!

@EricWF EricWF merged commit f274c50 into google:master Sep 10, 2018
@LebedevRI LebedevRI deleted the llvm branch September 10, 2018 20:31
@coveralls
Copy link

Coverage Status

Coverage remained the same at 87.635% when pulling bbf887d on LebedevRI:llvm into f090141 on google:master.

JBakamovic pushed a commit to JBakamovic/benchmark that referenced this pull request Dec 6, 2018
…with clang-cl" (google#673)

`MSVC` is true for clang-cl, but `"${CMAKE_CXX_COMPILER_ID}" STREQUAL
"MSVC"` is false, so we would enable -Wall, which means -Weverything
with clang-cl, and we get tons of undesired warnings.

Use the simpler condition to fix things.

Patch by: Reid Kleckner @rnk
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants