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

Fix DLL visibility of count_digits<4>(internal::uintptr_t) in format-inl.h #1147

Merged
merged 10 commits into from
May 7, 2019
Merged

Conversation

denchat
Copy link
Contributor

@denchat denchat commented May 7, 2019

Previously, test-format.exe linking failed because count_digits<4>(internal::uintptr_t) not found in the DLL.
Also suppresses warning of MSVC's _CRT_INSECURE_DEPRECATE functions from gtest.h included in custom-formatter-test.cc

I agree that my contributions are licensed under the {fmt} license, and agree to future changes to the licensing.

…on in format-inl.h

In FMT_SHARED mode, format-test.exe wants to DLL import `count_digits<4>(internal::uintptr_t)` explicit full specialization definition in format-inl.h
…on in format-inl.h

In FMT_SHARED mode, format-test.exe wants to DLL import `count_digits<4>(internal::uintptr_t)` explicit full specialization definition in format-inl.h
…e `strncpy`

Suppress MSVC warnings from `gtest.h` included.

Warning sample:
fmt-master\test\gtest\gtest.h(2873,10): warning: 'strncpy' is deprecated: This function or variable may be unsafe. Consider using strncpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [-Wdeprecated-declarations]
@vitaut
Copy link
Contributor

vitaut commented May 7, 2019

Thanks for the PR. Since count_digits is the implementation detail, its test should be moved to format-impl-test and it shouldn't be exported.

@@ -5,6 +5,12 @@
//
// For the license information refer to format.h.

#ifdef _MSC_VER
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below: I'd drop _MSC_VER_ altogether. There is no harm in always defining _CRT_SECURE_NO_WARNINGS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Anyway I notice format-imple-test is exempted when building DLL. When building DLL it includes format-inl.h in which cout_digits resides, I wonder if format-imple-test should be re-enabled also?

@vitaut vitaut merged commit 3fd134b into fmtlib:master May 7, 2019
@vitaut
Copy link
Contributor

vitaut commented May 7, 2019

Thanks!

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.

2 participants