Trying to suppress all clang-target-msvc test warning in CMAKE and other misc fixes #1151

denchat · 2019-05-08T19:22:11Z

Trying to suppress all clang-target-msvc warnings that are caused by gtest using CRT unsafe functions (_CRT_SECURE_NO_WARNINGS) and POSIX functions in CMAKE
Trying to automatically enable exceptions in MSVC in CMAKE file both fmt and test (Not sure if desirable?)
Fix char8_t in format-test.cc
Suppress clang-target-msvc warning in posix-mock-test.cc

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

…f [[no_discard]] begin() when in c++17

…larations Suppress warning _CRT_SECURE_NO_WARNINGS in MSVC and -Wdeprecated-declarations of POSIX functions in Clang target MSVC. Those functions are used by gtest.

User must manually turn on exceptions otherwise error occurs when compiling `format.cc` as below fmt-master\include\fmt/format-inl.h(891,3): error: cannot use 'try' with exceptions disabled FMT_TRY { ^

vitaut

Thanks a lot for the PR! A few comments inline.

vitaut · 2019-05-09T01:01:32Z

CMakeLists.txt

@@ -161,6 +161,9 @@ endif ()
 if (FMT_PEDANTIC)
  target_compile_options(fmt PRIVATE ${PEDANTIC_COMPILE_FLAGS})
 endif ()
+if (MSVC)
+  target_compile_options(fmt PRIVATE /EHsc)


Exceptions are enabled by default on MSVC. Why is this needed?

This would also force the library into a specific exception handling model, which would be undesirable.

@vitaut : My clang-target-msvc isn't defaulting /EHsc. Then I guess this should just trying to mirror /EHsc default in MSVC cl.exe. Perhaps...

if (MSVC)
should be changed into
if (MSVC AND CMAKE_CXX_COMPILER_ID MATCHES "Clang" )

@eliaskosunen Would you be okay with narrowing to clang-target-msvc for mirroring the default /EHsc above?

Should there be FMT_MSVC_EH option that explicitly add custom /EHxx?

I think, perhaps we can have FMT_MSVC_EH like this :

if (MSVC) if (FMT_MSVC_EH) set(FMT_CUSTOM_MSVC_EH ${FMT_MSVC_EH}) else () if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") set(FMT_CUSTOM_MSVC_EH /EHsc) else () set(FMT_CUSTOM_MSVC_EH) endif () endif () target_compile_options(fmt PRIVATE ${FMT_CUSTOM_MSVC_EH}) endif ()

What do you guys think?

I don't know why TravisCI evaluates MSVC true in apple platform and linux clang platform.
So I try to workaround with

if (MSVC AND WIN32) if (FMT_MSVC_EH) set(FMT_CUSTOM_MSVC_EH ${FMT_MSVC_EH}) else () if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") set(FMT_CUSTOM_MSVC_EH /EHsc) else () set(FMT_CUSTOM_MSVC_EH) endif () endif () target_compile_options(fmt PRIVATE ${FMT_CUSTOM_MSVC_EH}) endif ()

It seems that it's because my excessing endif, not MSVC weirdness. Fixed it.

My clang-target-msvc isn't defaulting /EHsc.

So what is the default in clang-target-msvc? Are exceptions disabled by default or enabled with a different setting?

C:\Users\User\AppData\Roaming\fmt>ninja -j 2 [1/47] Building CXX object CMakeFiles\fmt.dir\src\posix.cc.obj FAILED: CMakeFiles/fmt.dir/src/posix.cc.obj C:\PROGRA~1\LLVM\bin\clang-cl.exe /nologo -TP -DFMT_EXPORT -Dfmt_EXPORTS -IC:\Users\User\AppData\Roaming\fmt-master\include /MD /O2 /Ob2 /DNDEBUG -std:c++latest /showIncludes /FoCMakeFiles\fmt.dir\src\posix.cc.obj /FdCMakeFiles\fmt.dir\ -c C:\Users\User\AppData\Roaming\fmt-master\src\posix.cc In file included from C:\Users\User\AppData\Roaming\fmt-master\src\posix.cc:13: In file included from C:\Users\User\AppData\Roaming\fmt-master\include\fmt/posix.h:28: C:\Users\User\AppData\Roaming\fmt-master\include\fmt/format.h(121,10): error: cannot use 'throw' with exceptions disabled if (b) throw x; ^

It seems exceptions are disabled by default. Well, you are right, after take a look at => user manual, I found that users can explicitly state the configurations through very generous means. However, clang-cl.exe alone basically does not enable exceptions by default.

Nevertheless, if you think this compiler case wouldn't be worth for the added complexity, I will be fine with dropping these CMAKE changes for /EHsc.

I've removed conditional adding /EHsc from CMAKELists.txt in both root and test.

vitaut · 2019-05-09T01:05:07Z

test/format-test.cc

+// type in this mode. If this is the case __cpp_char8_t will be defined.
+#ifndef __cpp_char8_t
+// A UTF-8 code unit type.
+#define CHAR8_T fmt::char8_t


I suggest replacing a macro with a using declaration.

Done.

With c++11 using, it looks way way better now. Thank you.

vitaut · 2019-05-09T01:08:30Z

test/posix-mock-test.cc

@@ -461,6 +461,10 @@ struct LocaleMock {
 #  ifdef _MSC_VER
 #    pragma warning(push)
 #    pragma warning(disable : 4273)
+#    ifdef __clang__
+#      pragma clang diagnostic push
+#      pragma clang diagnostic ignored "-Winconsistent-dllimport"


Could you post the warnings?

C:\Users\User\AppData\Roaming\fmt>ninja -j 2 [4/47] Linking CXX shared library bin\fmt.dll Creating library fmt.lib and object fmt.exp [35/47] Building CXX object test\CMakeFiles\posix-mock-test.dir\posix-mock-test.cc.obj C:\Users\User\AppData\Roaming\fmt-master\test\posix-mock-test.cc(469,11): warning: '_create_locale' redeclared without 'dllimport' attribute: 'dllexport' attribute added [-Winconsistent-dllimport] _locale_t _create_locale(int category, const char* locale) { ^ C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\locale.h(105,32): note: previous declaration is here _ACRTIMP _locale_t __cdecl _create_locale( ^ C:\Users\User\AppData\Roaming\fmt-master\test\posix-mock-test.cc(473,6): warning: '_free_locale' redeclared without 'dllimport' attribute: 'dllexport' attribute added [-Winconsistent-dllimport] void _free_locale(_locale_t locale) { ^ C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\locale.h(110,27): note: previous declaration is here _ACRTIMP void __cdecl _free_locale( ^ C:\Users\User\AppData\Roaming\fmt-master\test\posix-mock-test.cc(477,8): warning: '_strtod_l' redeclared without 'dllimport' attribute: 'dllexport' attribute added [-Winconsistent-dllimport] double _strtod_l(const char* nptr, char** endptr, _locale_t locale) { ^ C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\stdlib.h(504,25): note: previous declaration is here _ACRTIMP double __cdecl _strtod_l( ^ 3 warnings generated.

somehow TravisCI evaluates MSVC true

So workaround with replacing If (MSVC) with if (MSVC AND WIN32)

Workaround by replacing if (MSVC) with if (MSVC AND WIN32)

MSVC by default doesn't enable /EHsc either. Exception setting depends on user configurations.

MSVC by default doesn't enable /EHsc also. Just keep it most simple.

…t64_t)` So we have to "export" here also.

vitaut · 2019-05-11T13:27:00Z

test/format-test.cc

@@ -2533,9 +2540,9 @@ TEST(FormatTest, EmphasisNonHeaderOnly) {
 TEST(FormatTest, CharTraitsIsNotAmbiguous) {
  // Test that we don't inject internal names into the std namespace.
  using namespace std;
-  char_traits<char>::char_type c;
+  FMT_MAYBE_UNUSED char_traits<char>::char_type c;


I suggest just doing

(void)c;

and removing the FMT_MAYBE_UNUSED macro because it is not very portable (requires C++17).

vitaut · 2019-05-11T13:27:08Z

test/format-test.cc

 #if __cplusplus >= 201103L
  std::string s;
-  begin(s);
+  FMT_MAYBE_UNUSED auto lval = begin(s);


vitaut · 2019-05-11T13:33:16Z

include/fmt/format-inl.h

@@ -683,7 +683,7 @@ template <int GRISU_VERSION> struct grisu_shortest_handler {
 };

 template <typename Double, FMT_ENABLE_IF_T(sizeof(Double) == sizeof(uint64_t))>
-FMT_FUNC bool grisu_format(Double value, buffer<char>& buf, int precision,
+FMT_FUNC FMT_API bool grisu_format(Double value, buffer<char>& buf, int precision,


FMT_API shouldn't be here because it's an internal symbol.

In DLL mode, grisu_format is refered by custom-formatter-test.cc.

C:\Users\User\AppData\Roaming\fmt>ninja -j 2 [3/47] Linking CXX shared library bin\fmt.dll Creating library fmt.lib and object fmt.exp [12/47] Linking CXX executable bin\custom-formatter-test.exe FAILED: bin/custom-formatter-test.exe cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe --intdir=test\CMakeFiles\custom-formatter-test.dir --rc=C:\PROGRA~2\WINDOW~4\10\bin\100177~1.0\x64\rc.exe --mt=C:\PROGRA~2\WINDOW~4\10\bin\100177~1.0\x64\mt.exe --manifests -- C:\PROGRA~2\MIB055~1\2019\BUILDT~1\VC\Tools\MSVC\1420~1.275\bin\Hostx64\x64\link.exe /nologo test\CMakeFiles\custom-formatter-test.dir\custom-formatter-test.cc.obj /out:bin\custom-formatter-test.exe /implib:test\custom-formatter-test.lib /pdb:bin\custom-formatter-test.pdb /version:0.0 /machine:x64 /INCREMENTAL:NO /subsystem:console test\test-main.lib test\gmock.lib fmt.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ." LINK: command "C:\PROGRA~2\MIB055~1\2019\BUILDT~1\VC\Tools\MSVC\1420~1.275\bin\Hostx64\x64\link.exe /nologo test\CMakeFiles\custom-formatter-test.dir\custom-formatter-test.cc.obj /out:bin\custom-formatter-test.exe /implib:test\custom-formatter-test.lib /pdb:bin\custom-formatter-test.pdb /version:0.0 /machine:x64 /INCREMENTAL:NO /subsystem:console test\test-main.lib test\gmock.lib fmt.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTFILE:bin\custom-formatter-test.exe.manifest" failed (exit code 1120) with the following output: custom-formatter-test.cc.obj : error LNK2019: unresolved external symbol "bool __cdecl fmt::v5::internal::grisu_format<double,0>(double,class fmt::v5::internal::buffer<char> &,int,unsigned int,int &)" (??$grisu_format@N$0A@@internal@v5@fmt@@YA_NNAEAV?$buffer@D@012@HIAEAH@Z) referenced in function "private: void __cdecl fmt::v5::basic_writer<class fmt::v5::back_insert_range<class fmt::v5::internal::buffer<char> > >::write_double<double>(double,struct fmt::v5::basic_format_specs<char> const &)" (??$write_double@N@?$basic_writer@V?$back_insert_range@V?$buffer@D@internal@v5@fmt@@@v5@fmt@@@v5@fmt@@AEAAXNAEBU?$basic_format_specs@D@12@@Z) bin\custom-formatter-test.exe : fatal error LNK1120: 1 unresolved externals [13/47] Building CXX object test\CMakeFiles\format-test.dir\format-test.cc.obj ninja: build stopped: subcommand failed.

Ah, you are right, it should be exported. However FMT_API is already on the declaration, so it shouldn't be needed here:

fmt/include/fmt/format.h

Line 1125 in ea2976e

FMT_API bool grisu_format(Double, buffer<char>&, int, unsigned, int&);

. I wonder why MSVC is complaining.

Anyway, if it fixes the problem, it's fine to keep FMT_API here but please remove FMT_FUNC then.

I've also removed FMT_API from the corresponding declaration in format.h

template <typename Double, FMT_ENABLE_IF(sizeof(Double) == sizeof(uint64_t))> FMT_API bool grisu_format(Double, buffer<char>&, int, unsigned, int&);

Now it becomes

template <typename Double, FMT_ENABLE_IF(sizeof(Double) == sizeof(uint64_t))> bool grisu_format(Double, buffer<char>&, int, unsigned, int&);

Sorry I didn't refresh this page. Now I'm looking at it.

In format.h, it's been rolled back to

template <typename Double, FMT_ENABLE_IF(sizeof(Double) == sizeof(uint64_t))> FMT_API bool grisu_format(Double, buffer<char>&, int, unsigned, int&);

In format-inl.h, remove FMT_FUNC and add FMT_API

template <typename Double, typename std::enable_if< sizeof(Double) == sizeof(uint64_t), int>::type> FMT_API bool grisu_format(Double value, buffer<char>& buf, int precision, unsigned options, int& exp) {

In DLL mode, format-test.cc refers to

internal::count_digits<4,struct fmt::v5::internal::uintptr::uintptr_t>(struct fmt::v5::internal::uintptr::uintptr_t)

The linker can not find symbol :

C:\Users\User\AppData\Roaming\fmt>ninja -j 2 [3/47] Linking CXX shared library bin\fmt.dll Creating library fmt.lib and object fmt.exp [15/47] Linking CXX executable bin\format-test.exe FAILED: bin/format-test.exe cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe --intdir=test\CMakeFiles\format-test.dir --rc=C:\PROGRA~2\WINDOW~4\10\bin\100177~1.0\x64\rc.exe --mt=C:\PROGRA~2\WINDOW~4\10\bin\100177~1.0\x64\mt.exe --manifests -- C:\PROGRA~2\MIB055~1\2019\BUILDT~1\VC\Tools\MSVC\1420~1.275\bin\Hostx64\x64\link.exe /nologo test\CMakeFiles\format-test.dir\format-test.cc.obj /out:bin\format-test.exe /implib:test\format-test.lib /pdb:bin\format-test.pdb /version:0.0 /machine:x64 /INCREMENTAL:NO /subsystem:console test\test-main.lib test\gmock.lib fmt.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ." LINK: command "C:\PROGRA~2\MIB055~1\2019\BUILDT~1\VC\Tools\MSVC\1420~1.275\bin\Hostx64\x64\link.exe /nologo test\CMakeFiles\format-test.dir\format-test.cc.obj /out:bin\format-test.exe /implib:test\format-test.lib /pdb:bin\format-test.pdb /version:0.0 /machine:x64 /INCREMENTAL:NO /subsystem:console test\test-main.lib test\gmock.lib fmt.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTFILE:bin\format-test.exe.manifest" failed (exit code 1120) with the following output: format-test.cc.obj : error LNK2019: unresolved external symbol "int __cdecl fmt::v5::internal::count_digits<4,struct fmt::v5::internal::uintptr::uintptr_t>(struct fmt::v5::internal::uintptr::uintptr_t)" (??$count_digits@$03Uuintptr_t@uintptr@internal@v5@fmt@@@internal@v5@fmt@@YAHUuintptr_t@uintptr@012@@Z) referenced in function "public: void __cdecl fmt::v5::basic_writer<class fmt::v5::back_insert_range<class fmt::v5::internal::buffer<char> > >::write_pointer<struct fmt::v5::internal::uintptr::uintptr_t>(struct fmt::v5::internal::uintptr::uintptr_t,struct fmt::v5::align_spec const *)" (??$write_pointer@Uuintptr_t@uintptr@internal@v5@fmt@@@?$basic_writer@V?$back_insert_range@V?$buffer@D@internal@v5@fmt@@@v5@fmt@@@v5@fmt@@QEAAXUuintptr_t@uintptr@internal@12@PEBUalign_spec@12@@Z) bin\format-test.exe : fatal error LNK1120: 1 unresolved externals [16/47] Building CXX object test\CMakeFiles\assert-test.dir\assert-test.cc.obj ninja: build stopped: subcommand failed.

In format-test.cc , if I comment out

TEST(WriterTest, WriteUIntPtr) { memory_buffer buf; fmt::writer writer(buf); writer.write_pointer(fmt::internal::bit_cast<fmt::internal::uintptr_t>( reinterpret_cast<void*>(0xface)), FMT_NULL); EXPECT_EQ("0xface", to_string(buf)); }

All test are passed.

Oh, it's already been fixed. I didn't see. Thank you.

when sizeof(Double) == sizeof(uint64_t)

When sizeof(Double) == sizeof(uint64_t)

vitaut · 2019-05-11T18:49:21Z

Thanks!

denchat added 4 commits May 9, 2019 01:49

Fix conditional char8_t from format.h and fix -Wunused-result o…

c575d56

…f [[no_discard]] begin() when in c++17

Suppress -Winconsistent-dllimport when in clang-target-msvc

28588a9

Suppress warning _CRT_SECURE_NO_WARNINGS in MSVC and -Wdeprecated-dec…

abb3050

…larations Suppress warning _CRT_SECURE_NO_WARNINGS in MSVC and -Wdeprecated-declarations of POSIX functions in Clang target MSVC. Those functions are used by gtest.

Turn on exceptions automatically in MSVC

63d6445

User must manually turn on exceptions otherwise error occurs when compiling `format.cc` as below fmt-master\include\fmt/format-inl.h(891,3): error: cannot use 'try' with exceptions disabled FMT_TRY { ^

denchat changed the title ~~Trying to suppress all clang-target-msvc warning in CMAKE and other misc fixes~~ Trying to suppress all clang-target-msvc test warning in CMAKE and other misc fixes May 8, 2019

vitaut requested changes May 9, 2019

View reviewed changes

denchat added 24 commits May 9, 2019 12:22

Use 'using' to provide char8_t type if neccessary as Victor's suggestion

c8cbf57

Narrowing scope to clang-target-msvc for mirroring cl default /EHsc

d75c98b

Narrowing /EHsc to clang-target-msvc only

d7b83ff

missing that semicolon!

073571a

Receive FMT_MSVC_EH from user

eb420be

Receive FMT_MSVC_EH from user

b17216a

Fix excessing parenthesis

e7e48c3

Fix excessing parenthesis

c079096

Fix AppleClang match

9d0de74

Fix AppleClang match

6593a07

Try workaround with WIN32

bdc4929

somehow TravisCI evaluates MSVC true

Try workaround with WIN32

28b559e

Because TravisCI evaluates MSVC true in linux and apple

133af4a

So workaround with replacing If (MSVC) with if (MSVC AND WIN32)

Because TravisCI evaluates MSVC true in linux and apple

39a140a

Workaround by replacing if (MSVC) with if (MSVC AND WIN32)

Bring _VARIADIC_MAX=10 out of MSVC AND WIN32 context

032ac28

gmock PUBLIC

8d8f7b1

Update CMakeLists.txt

473ccd0

Update CMakeLists.txt

ade161d

Update CMakeLists.txt

ff44d1a

Not sure about MSVC, test travis again

03d7600

Turns out that I misunderstood MSVC

b6db4b8

Add conditional defined FMT_MAYBE_UNUSED

9155a22

Use [[maybe_unused]] to suppress -Wunused-variable

eaaa44c

Delete selective adding /EHsc. Leave it to users.

02e472d

MSVC by default doesn't enable /EHsc either. Exception setting depends on user configurations.

denchat added 2 commits May 11, 2019 18:50

Rollback

f1fd5ac

MSVC by default doesn't enable /EHsc also. Just keep it most simple.

Because grisu_format does "import" when `sizeof(Double) == sizeof(uin…

659567c

…t64_t)` So we have to "export" here also.

vitaut requested changes May 11, 2019

View reviewed changes

denchat added 6 commits May 11, 2019 23:37

Rollback

889f904

(void)X; suppresses -Wunused-variable

8b3227f

bool grisu_format is not supposed to be imported/exported

6d0343f

when sizeof(Double) == sizeof(uint64_t)

grisu_format is not supposed to be imported/exported

166b79d

When sizeof(Double) == sizeof(uint64_t)

Rollback

3b4928a

Remove FMT_FUNC, mark FMT_API to export

e422dad

vitaut merged commit f4dfd6e into fmtlib:master May 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trying to suppress all clang-target-msvc test warning in CMAKE and other misc fixes #1151

Trying to suppress all clang-target-msvc test warning in CMAKE and other misc fixes #1151

denchat commented May 8, 2019 •

edited

Loading

vitaut left a comment

vitaut May 9, 2019

eliaskosunen May 9, 2019

denchat May 9, 2019

denchat May 9, 2019 •

edited

Loading

denchat May 9, 2019 •

edited

Loading

denchat May 9, 2019

vitaut May 9, 2019 •

edited

Loading

denchat May 10, 2019 •

edited

Loading

denchat May 11, 2019

vitaut May 9, 2019

denchat May 9, 2019

vitaut May 9, 2019

denchat May 9, 2019

vitaut May 11, 2019

vitaut May 11, 2019

vitaut May 11, 2019

denchat May 11, 2019

vitaut May 11, 2019

vitaut May 11, 2019

denchat May 11, 2019

denchat May 11, 2019

denchat May 11, 2019

denchat May 11, 2019

denchat May 11, 2019

vitaut commented May 11, 2019

Trying to suppress all clang-target-msvc test warning in CMAKE and other misc fixes #1151

Trying to suppress all clang-target-msvc test warning in CMAKE and other misc fixes #1151

Conversation

denchat commented May 8, 2019 • edited Loading

vitaut left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denchat May 9, 2019 • edited Loading

Choose a reason for hiding this comment

denchat May 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitaut May 9, 2019 • edited Loading

Choose a reason for hiding this comment

denchat May 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitaut commented May 11, 2019

denchat commented May 8, 2019 •

edited

Loading

denchat May 9, 2019 •

edited

Loading

denchat May 9, 2019 •

edited

Loading

vitaut May 9, 2019 •

edited

Loading

denchat May 10, 2019 •

edited

Loading