opentelemetry-cpp
opentelemetry-cpp copied to clipboard
Compiler warnings clean-up
Repo has 26 compiler warnings with vs2019 in:
- tracez_processor_test
- ostream_metrics_test
- opentelemetry_exporter_ostream_metrics
Most of these are about potentially unsafe truncating type casts:
'initializing': conversion from 'float' to 'int', possible loss of data'=': conversion from 'double' to 'int', possible loss of data'=': conversion from 'size_t' to 'int', possible loss of data'initializing': conversion from 'size_t' to 'int', possible loss of data'return': conversion from 'double' to 'T', possible loss of data
Recommendation is to fix those warnings. And change our build flags to All or Level 4 :
| Compiler | Compiler Flags |
|---|---|
| gcc | -Wall -Wextra |
| clang | -Wall -Wextra |
| MSVC | /W4 |
Plus treat all warnings as errors by default across all compilers, to keep it at zero warnings at all times. It is gonna be progressively harder to clean it later if we don't keep it at zero from the very beginning.
Maybe it's better to remove warning from protobuf? There is a lot warnings which are come from protobuf headers, for example:
[build] D:\prebuilt\vcpkg\installed\x64-windows\include\google/protobuf/stubs/logging.h(102,23): warning C4251: “google::protobuf::internal::LogMessage::message_”: class“std::basic_string<char,std::char_traits<char>,std::allocator<char>>”需要有 dll 接口由 class“google::protobuf::internal::LogMessage”的客户端使用 [D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\opentelemetry_proto.vcxproj]
[build] C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29910\include\xstring(4648): message : 参见“std::basic_string<char,std::char_traits<char>,std::allocator<char>>”的声明 [D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\opentelemetry_proto.vcxproj]
[build] otlp_foo_library.vcxproj -> D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\examples\otlp\Debug\otlp_foo_library.lib
[build] load_plugin_example.vcxproj -> D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\examples\plugin\load\Debug\load_plugin_example.exe
[build] D:\prebuilt\vcpkg\installed\x64-windows\include\google/protobuf/io/coded_stream.h(1250,64): warning C4251: “google::protobuf::io::CodedOutputStream::default_serialization_deterministic_”: struct“std::atomic<bool>”需要有 dll 接口由 class“google::protobuf::io::CodedOutputStream”的客户端使用 [D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\opentelemetry_proto.vcxproj]
[build] C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29910\include\atomic(2777): message : 参见“std::atomic<bool>”的声明 [D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\opentelemetry_proto.vcxproj]
[build] D:\prebuilt\vcpkg\installed\x64-windows\include\google/protobuf/arena_impl.h(325,37): warning C4251: “google::protobuf::internal::ArenaImpl::threads_”: struct“std::atomic<google::protobuf::internal::SerialArena *>”需要有 dll 接口由 class“google::protobuf::internal::ArenaImpl”的客户端使用 [D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\opentelemetry_proto.vcxproj]
[build] D:\prebuilt\vcpkg\installed\x64-windows\include\google/protobuf/arena_impl.h(325): message : 参见“std::atomic<google::protobuf::internal::SerialArena *>”的声明 [D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\opentelemetry_proto.vcxproj]
[build] D:\prebuilt\vcpkg\installed\x64-windows\include\google/protobuf/arena_impl.h(326,34): warning C4251: “google::protobuf::internal::ArenaImpl::hint_”: struct“std::atomic<google::protobuf::internal::SerialArena *>”需要有 dll 接口由 class“google::protobuf::internal::ArenaImpl”的客户端使用 [D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\opentelemetry_proto.vcxproj]
[build] D:\prebuilt\vcpkg\installed\x64-windows\include\google/protobuf/arena_impl.h(325): message : 参见“std::atomic<google::protobuf::internal::SerialArena *>”的声明 [D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\opentelemetry_proto.vcxproj]
[build] D:\prebuilt\vcpkg\installed\x64-windows\include\google/protobuf/arena_impl.h(327,39): warning C4251: “google::protobuf::internal::ArenaImpl::space_allocated_”: struct“std::atomic<unsigned __int64>”需要有 dll 接口由 class“google::protobuf::internal::ArenaImpl”的客户端使用 [D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\opentelemetry_proto.vcxproj]
[build] 正在创建库 D:/workspace/github/owent-contrib/opentelemetry/opentelemetry-cpp/build_jobs_cmake_tools/examples/plugin/plugin/Debug/example_plugin.lib 和对象 D:/workspace/github/owent-contrib/opentelemetry/opentelemetry-cpp/build_jobs_cmake_tools/examples/plugin/plugin/Debug/example_plugin.exp
[build] C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29910\include\atomic(2789): message : 参见“std::atomic<unsigned __int64>”的声明 [D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\opentelemetry_proto.vcxproj]
[build] dynamic_load_test.vcxproj -> D:\workspace\github\owent-contrib\opentelemetry\opentelemetry-cpp\build_jobs_cmake_tools\api\test\plugin\Debug\dynamic_load_test.exe
Can I create a PR to remove some warnings from protobuf?
@owt5008137 Thanks for reporting these warnings. It would be definitely helpful if you raise PR for fixing these warnings. Please go ahead and raise a PR.
@owt5008137 - agree with removing from Protobuf. I like the way how you handled it, esp. push and pop . Thank you!!!
This issue was marked as stale due to lack of activity. It will be closed in 7 days if no furthur activity occurs.
This issue was marked as stale due to lack of activity. It will be closed in 7 days if no furthur activity occurs.
This issue was marked as stale due to lack of activity. It will be closed in 7 days if no furthur activity occurs.
Closed as inactive. Feel free to reopen if this is still an issue.
Reopening, as we still have warnings.
@lalitb do we plan to enable /W4 or related options?
@ThomsonTan Yes, we should be enabling this for all platforms once warnings are cleaned-up.