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

MSVC C5046 warning is unavailable in MSVC 2015. #2226

Merged
merged 1 commit into from Apr 11, 2019

Conversation

Projects
None yet
4 participants
@davidben
Copy link
Contributor

commented Apr 10, 2019

Per the MSVC documentation, the warning is new as of Visual Studio 2017, version 15.8.
https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/c5046?view=vs-2019

GTest users building on MSVC 2015 or older versions of 2017 will, when C4616 is enabled, see a warning like:

[...]gtest-matchers.h(53): error C2220: warning treated as error - no 'object' file generated
[...]gtest-matchers.h(53): warning C4619: #pragma warning: there is no warning number '5046'

Guard the mention of 5046 by an _MSC_VER check. VS2017 15.8 corresponds to an _MSC_VER of 1915.
https://docs.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=vs-2019

VS2017 15.7 is not very old (15.7 was released August 2018 and 15.8 was released November 2018), and I believe VS2015 is still covered under Abseil's support guarantees. (BoringSSL have some consumers who sadly still support VS2015, so we have to apply this patch to update googletest.)

MSVC C5046 warning is unavailable in MSVC 2015.
Per the MSVC documentation the warning is new as of Visual Studio 2017,
version 15.8.
https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/c5046?view=vs-2019

GTest users building on MSVC 2015 or older versions of 2017 will, when
C4616 is enabled, see a warning like:

[...]gtest-matchers.h(53): error C2220: warning treated as error - no 'object' file generated
[...]gtest-matchers.h(53): warning C4619: #pragma warning: there is no warning number '5046'

Guard the mention of 5046 by an _MSC_VER check. VS2017 15.8 corresponds
to an _MSC_VER of 1915.
https://docs.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=vs-2019

@googlebot googlebot added the cla: yes label Apr 10, 2019

@pwnall

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

/cc @gennadiycivil This would be helpful to google/boringssl which is used (at least) in Chrome. PTAL?

agl added a commit to google/boringssl that referenced this pull request Apr 10, 2019

Update third_party/googletest.
The new version of googletest deprecates INSTANTIATE_TEST_CASE_P in
favor of INSTANTIATE_TEST_SUITE_P, so apply the change.

This requires blacklisting C4628 on MSVC 2015 which says about digraphs
given foo<::std::tuple<...>>. Disable that warning. Digraphs are not
useful and C++11 apparently explicitly disambiguates that.

It also requires applying
google/googletest#2226, to deal with a warning
in older MSVC.

Update-Note: Consumers using BoringSSL with their own copy of googletest
must ensure googletest was updated to a version from 2019-01-03 or
later for INSTANTIATE_TEST_SUITE_P to work. (I believe all relevant
consumers are fine here. If anyone can't update googletest and is
building BoringSSL tests, building with
-DINSTANTIATE_TEST_SUITE_P=INSTANTIATE_TEST_CASE_P would work as
workaround.)

Bug: chromium:936651
Change-Id: I23ada8de34a53131cab88a36a88d3185ab085c64
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35504
Reviewed-by: Adam Langley <agl@google.com>

@gennadiycivil gennadiycivil merged commit 8e9297b into google:master Apr 11, 2019

3 checks passed

cla/google All necessary CLAs are signed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

gennadiycivil added a commit that referenced this pull request Apr 11, 2019

Merge pull request #2226 from davidben:msvc-5046
PiperOrigin-RevId: 243121568
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.