-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Trying to suppress all clang-target-msvc test warning in CMAKE and other misc fixes #1151
Changes from 30 commits
c575d56
28588a9
abb3050
63d6445
c8cbf57
d75c98b
d7b83ff
073571a
eb420be
b17216a
e7e48c3
c079096
9d0de74
6593a07
bdc4929
28b559e
133af4a
39a140a
032ac28
8d8f7b1
473ccd0
ade161d
ff44d1a
03d7600
b6db4b8
9155a22
eaaa44c
02e472d
f1fd5ac
659567c
889f904
8b3227f
6d0343f
166b79d
3b4928a
e422dad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2491,18 +2491,25 @@ TEST(FormatTest, FmtStringInTemplate) { | |
|
||
#endif // FMT_USE_CONSTEXPR | ||
|
||
// C++20 feature test, since r346892 Clang considers char8_t a fundamental | ||
// type in this mode. If this is the case __cpp_char8_t will be defined. | ||
#ifndef __cpp_char8_t | ||
// Locally provide type char8_t defined in format.h | ||
using fmt::char8_t; | ||
#endif | ||
|
||
TEST(FormatTest, ConstructU8StringViewFromCString) { | ||
fmt::u8string_view s("ab"); | ||
EXPECT_EQ(s.size(), 2u); | ||
const fmt::char8_t* data = s.data(); | ||
const char8_t* data = s.data(); | ||
EXPECT_EQ(data[0], 'a'); | ||
EXPECT_EQ(data[1], 'b'); | ||
} | ||
|
||
TEST(FormatTest, ConstructU8StringViewFromDataAndSize) { | ||
fmt::u8string_view s("foobar", 3); | ||
EXPECT_EQ(s.size(), 3u); | ||
const fmt::char8_t* data = s.data(); | ||
const char8_t* data = s.data(); | ||
EXPECT_EQ(data[0], 'f'); | ||
EXPECT_EQ(data[1], 'o'); | ||
EXPECT_EQ(data[2], 'o'); | ||
|
@@ -2513,7 +2520,7 @@ TEST(FormatTest, U8StringViewLiteral) { | |
using namespace fmt::literals; | ||
fmt::u8string_view s = "ab"_u; | ||
EXPECT_EQ(s.size(), 2u); | ||
const fmt::char8_t* data = s.data(); | ||
const char8_t* data = s.data(); | ||
EXPECT_EQ(data[0], 'a'); | ||
EXPECT_EQ(data[1], 'b'); | ||
EXPECT_EQ(format("{:*^5}"_u, "🤡"_u), "**🤡**"_u); | ||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest just doing
and removing the |
||
#if __cplusplus >= 201103L | ||
std::string s; | ||
begin(s); | ||
FMT_MAYBE_UNUSED auto lval = begin(s); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
#endif | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you post the warnings? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
# endif | ||
|
||
_locale_t _create_locale(int category, const char* locale) { | ||
return LocaleMock::instance->newlocale(category, locale, 0); | ||
|
@@ -473,6 +477,9 @@ void _free_locale(_locale_t locale) { | |
double _strtod_l(const char* nptr, char** endptr, _locale_t locale) { | ||
return LocaleMock::instance->strtod_l(nptr, endptr, locale); | ||
} | ||
# ifdef __clang__ | ||
# pragma clang diagnostic pop | ||
# endif | ||
# pragma warning(pop) | ||
# endif | ||
|
||
|
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.
FMT_API
shouldn't be here because it's an internal symbol.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.
In DLL mode, grisu_format is refered by custom-formatter-test.cc.
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.
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
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.
Anyway, if it fixes the problem, it's fine to keep
FMT_API
here but please removeFMT_FUNC
then.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.
I've also removed FMT_API from the corresponding declaration in
format.h
Now it becomes
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.
Sorry I didn't refresh this page. Now I'm looking at it.
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.
In
format.h
, it's been rolled back toIn
format-inl.h
, remove FMT_FUNC and add FMT_APIThere 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.
In DLL mode,
format-test.cc
refers toThe linker can not find symbol :
In format-test.cc , if I comment out
All test are passed.
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.
Oh, it's already been fixed. I didn't see. Thank you.