[reflection] fix type_string that lose const #874
Conversation
|
|
| #if defined(_MSC_VER) | ||
| constexpr std::size_t npos = str.find_first_of(" ", pos); | ||
| if constexpr (npos != std::string_view::npos) | ||
| return str.substr(npos + 1, next1 - npos - 1); | ||
| else | ||
| return str.substr(pos, next1 - pos); | ||
| #else | ||
| return str.substr(pos, next1 - pos); | ||
| #endif |
There was a problem hiding this comment.
remove code for msvc?
have you tested on windows?
There was a problem hiding this comment.
yes, I have tested on windows with msvc and clang-cl
| // CHECK(type_string<int&>() == "int &"); | ||
| // CHECK(type_string<int&&>() == "int &&"); | ||
| // CHECK(type_string<const int&>() == "const int &"); | ||
| // CHECK(type_string<const int&&>() == "const int &&"); | ||
| // CHECK(type_string<test_type_string::struct_test>() == "test_type_string::struct_test"); | ||
| // CHECK(type_string<test_type_string::struct_test&>() == "test_type_string::struct_test &"); | ||
| // CHECK(type_string<test_type_string::struct_test&&>() == "test_type_string::struct_test &&"); | ||
| // CHECK(type_string<const test_type_string::struct_test>() == "const test_type_string::struct_test"); | ||
| // CHECK(type_string<const test_type_string::struct_test&>() == "const test_type_string::struct_test &"); | ||
| // CHECK(type_string<const test_type_string::struct_test&&>() == "const test_type_string::struct_test &&"); | ||
| // CHECK(type_string<test_type_string::class_test>() == "test_type_string::class_test"); | ||
| // CHECK(type_string<test_type_string::class_test&>() == "test_type_string::class_test &"); | ||
| // CHECK(type_string<test_type_string::class_test&&>() == "test_type_string::class_test &&"); |
There was a problem hiding this comment.
but I'm wondering is this the right way, because the output is not exactly the same for all compilers, on my win11: clang:
- there is a space between int and &
- there is no class or struct for class name
CHECK(type_string<int&>() == "int &"); CHECK(type_string<int&&>() == "int &&"); CHECK(type_string<const int&>() == "const int &"); CHECK(type_string<const int&&>() == "const int &&"); CHECK(type_string<test_type_string::struct_test>() == "test_type_string::struct_test"); CHECK(type_string<test_type_string::struct_test&>() == "test_type_string::struct_test &"); CHECK(type_string<test_type_string::struct_test&&>() == "test_type_string::struct_test &&"); CHECK(type_string<const test_type_string::struct_test>() == "const test_type_string::struct_test"); CHECK(type_string<const test_type_string::struct_test&>() == "const test_type_string::struct_test &"); CHECK(type_string<const test_type_string::struct_test&&>() == "const test_type_string::struct_test &&"); CHECK(type_string<test_type_string::class_test>() == "test_type_string::class_test"); CHECK(type_string<test_type_string::class_test&>() == "test_type_string::class_test &"); CHECK(type_string<test_type_string::class_test&&>() == "test_type_string::class_test &&"); CHECK(type_string<const test_type_string::class_test>() == "const test_type_string::class_test"); CHECK(type_string<const test_type_string::class_test&>() == "const test_type_string::class_test &"); CHECK(type_string<const test_type_string::class_test&&>() == "const test_type_string::class_test &&");msvc:
- there is no space between int and &
- there is a class or struct for class name
CHECK(type_string<int&>() == "int&"); CHECK(type_string<int&&>() == "int&&"); CHECK(type_string<const int&>() == "const int&"); CHECK(type_string<const int&&>() == "const int&&"); CHECK(type_string<test_type_string::struct_test>() == "struct test_type_string::struct_test"); CHECK(type_string<test_type_string::struct_test&>() == "struct test_type_string::struct_test&"); CHECK(type_string<test_type_string::struct_test&&>() == "struct test_type_string::struct_test&&"); CHECK(type_string<const test_type_string::struct_test>() == "const struct test_type_string::struct_test"); CHECK(type_string<const test_type_string::struct_test&>() == "const struct test_type_string::struct_test&"); CHECK(type_string<const test_type_string::struct_test&&>() == "const struct test_type_string::struct_test&&"); CHECK(type_string<test_type_string::class_test>() == "class test_type_string::class_test"); CHECK(type_string<test_type_string::class_test&>() == "class test_type_string::class_test&"); CHECK(type_string<test_type_string::class_test&&>() == "class test_type_string::class_test&&"); CHECK(type_string<const test_type_string::class_test>() == "const class test_type_string::class_test"); CHECK(type_string<const test_type_string::class_test&>() == "const class test_type_string::class_test&"); CHECK(type_string<const test_type_string::class_test&&>() == "const class test_type_string::class_test&&");
when the T has & or &&, or is a class or struct, it's not exactly the same, see above^^^, so this part of the code is commented out, is this behavior ok?
There was a problem hiding this comment.
这些测试之前是能测试过的吧，为什么PR里就不过了？
There was a problem hiding this comment.
/home/runner/work/yalantinglibs/yalantinglibs/src/metric/tests/test_metric.cpp:1970: ERROR: CHECK( count == 1 ) is NOT correct!
5: values: CHECK( 3 == 1 )
这个metric看着也没用reflection里的东西吧
There was a problem hiding this comment.
这个测试先忽略，和定时器有关，一般重试一下就好了，和现在的pr无关。
There was a problem hiding this comment.
第一个问题：clang和gcc的输出内容是否有差异？ 第二个问题：能不能统一输出一种格式，以clang的输出为基准，msvc等编译器都按照这个格式输出；
- 如果是引用的类型那么clang的输出比gcc的多一个空格
- 我觉得应该统一一下，不过这样的话就需要把字符串切开再合并，以clang为准，那么gcc的输出要在字符串中新加一个空格，msvc的输出要加空格并且把字符串里的struct或者class关键字删掉，不知道咋在constexpr里操作string的我看看
- 其实现在的实现也不是跨平台只不过没写测试用例，见https://www.godbolt.org/z/nzxGGWqPE
There was a problem hiding this comment.
嗯，刚好这次完善一下。编译期操作字符串可以参考ylt的这个代码：https://github.com/alibaba/yalantinglibs/blob/main/include/ylt/easylog/record.hpp#L277
There was a problem hiding this comment.
我试了下这个refvalue::meta_string，如果再返回std::string_view地址就失效了，是不是应该要改成返回refvalue::meta_string呢 https://www.godbolt.org/z/q4sc9zc97
There was a problem hiding this comment.
用refvalue::meta_string更新了，现在是以gcc的输出为准，写起来简单点
|
for detail, goto summary download Artifacts |
|
要兼容gcc9 C++17 |
|
for detail, goto summary download Artifacts |
reflection不是c++20的吗，是支持gcc9 C++17，但是不支持别的编译器的c++17是吧 |
reflection 也是支持C++17的，C++17 下需要用宏，20 才不需要用宏。 不只是gcc，也要支持其它的编译器的c++17。 |
OK，那这个readme得改一下 |
这里说的是序列化库吧，还没有提到反射库。 |
|
我感觉PR的代码搞复杂了，似乎几行代码就可以了，不需要做太多修改。 template <typename T>
inline constexpr auto type_string() {
constexpr std::string_view sample = get_raw_name<int>();
constexpr size_t pos = sample.find("int");
constexpr std::string_view str = get_raw_name<T>();
constexpr auto next1 = str.rfind(sample[pos + 3]);
auto sub = str.substr(pos, next1 - pos);
refvalue::meta_string<next1 - pos> copy_str;
for (size_t i = 0; i < sub.size(); i++) {
copy_str[i] = sub[i];
}
#if defined(_MSC_VER)
constexpr std::size_t npos = str.find_first_of(" ", pos);
if constexpr (npos != std::string_view::npos)
return str.substr(npos + 1, next1 - npos - 1);
else
return str.substr(pos, next1 - pos);
#else
#if defined(__clang__)
if constexpr (str.find('&') != std::string_view::npos) {
size_t ref_pos = copy_str.rfind('&');
size_t space_pos = copy_str.rfind(' ');
for (size_t i = space_pos; i < ref_pos; i++) {
copy_str[i] = '&';
}
return copy_str.template substr<0, next1 - pos - 1>();
}
else {
return copy_str;
}
#else
return copy_str;
#endif
#endif
}思路是这样的：gcc和msvc的引号&是没有空格的，只有clang才有一个多余的空格，所以只要把clang的&整体向前移动一位即可，最后多余的一个字符做substr就行了，且只需要处理clang下有&字符的情况即可，其它情况也不需要处理。 这样就不需要修改meta string的代码了。 我测试是没问题的。 你按照我这个思路再适配一下msvc。 |
这个readme字面意思：如果你的编译器不支持c++20，那么只会编译序列化库。他的潜台词就是别的库(比如反射库)不支持c++17，我也是这么认为的。既然反射库是支持c++17的，我感觉这里有歧义 |
ok，那可以和pr一起更新一下readme。 |
理解你意思了，我直接在这儿先remove再substr，不过要去掉所有的，总结一下：
|
没有。不过msv需要substr吗？ |
|
行，你再改改，保持代码的可读性和效率就行。 |
|
又发现了几个不同编译器不一致的地方，见https://www.godbolt.org/z/aqPKKaoPM
总的，我觉得试图统一输出在目前这种实现下是不可能的了，我们不可能百分百的覆盖所有的情况... |
要不这样，只处理gcc clang 引用多了一个空格的问题，处理之后gcc和clang除了字符类型之外，输出几乎是一样的，甚至还可以针对gcc string 做一个特殊处理统一输出？msvc 差异太大了就不统一了。 另外一个方案是把const, volatile, reference 都去掉，只取纯名字，这样也有利于处理。 |
|
想了一下，还是不追求跨平台一致吧，目前没有这种需求。你只要把漏掉的const, volatile, reference 加上就好了，最好还是返回std::string_view, 用起来更方便。 |
|
嗯嗯好的，我在测试里按编译器区分了 |
|
for detail, goto summary download Artifacts |
|
for detail, goto summary download Artifacts |
Why
Fixes #872
What is changing
raw_name = fixed length prefix + type_name + fixed length suffix
so we can get prefix_length and suffix_length first, then substr
Example
but I'm wondering is this the right way, because the output is not exactly the same for all compilers,
on my win11:
clang:
msvc: