VS2017 Only Build Break in obj/third_party/webrtc/call/call/call.obj |
||
Issue description>ninja -C out\Debug obj/third_party/webrtc/call/call/call.obj ninja: Entering directory `out\Debug' [ 1/1 - 1 processes @ 0.4/s : 2.504s ] CXX obj/third_party/webrtc/call/call/call.obj FAILED: obj/third_party/webrtc/call/call/call.obj ninja -t msvc -e environment.x86 -- "d:\src\third_party\depot_tools\win_toolchain\vs_files\1f52d730755ac72dddaf121b73c9d6fd5c24ddf8\vc\tools\msvc\14.11.25503\bin\hostx64\x86/cl.exe" /nologo /showIncludes @obj/third_party/webrtc/call/call/call.obj.rsp /c ../../third_party/webrtc/call/call.cc /Foobj/third_party/webrtc/call/call/call.obj /Fd"obj/third_party/webrtc/call/call_cc.pdb" D:\src\third_party\webrtc/modules/video_coding/sequence_number_util.h(90): error C2672: 'rtc::SafeLt': no matching overloaded function found D:\src\third_party\webrtc/modules/video_coding/sequence_number_util.h(118): note: see reference to class template instantiation 'webrtc::SeqNumUnwrapper<T,M>' being compiled ninja: build stopped: subcommand failed. VC2015 = Okay VC2017 = Broken VC2017 Prerelease = Broken Clang = Okay An isolated test case suggests that the static_assert should be playing okay. VC2017 isn't outputting the full template instantiation, which makes it tricky to diagnose the issue.
,
Aug 8 2017
Hmmm. My repro has bugs in it (LeOp versus LtOp) - maybe that's the problem with the original code. Please hold.
,
Aug 8 2017
Having thought about it some more...
The problem happens when VC++ 2017 is analyzing SeqNumUnwrapper when it has not been instantiated. And, the problem has to do with std::numeric_limits<T>::max(). If this expression is replaced with an integer constant then the code compiles. It appears that VC++ 2017 has problems evaluating std::numeric_limits<T>::max() when T is not known. I tried casting the result of std::numeric_limits<T>::max() to uint64_t but that didn't help.
Here are some workarounds that do work:
static_assert(
std::is_unsigned<T>::value &&
rtc::SafeLt(sizeof(T),
sizeof(uint64_t)),
"Type unwrapped must be an unsigned integer smaller than uint64_t.");
static_assert(
std::is_unsigned<T>::value &&
std::numeric_limits<T>::max() < std::numeric_limits<uint64_t>::max(),
"Type unwrapped must be an unsigned integer smaller than uint64_t.");
Since there is no need for the SafeLt indirection I think I'll land the second workaround.
,
Aug 8 2017
,
Aug 9 2017
Independent of this compiler issue, the rtc code should really be replaced with the equivalent functionality from base/numerics. Just looking at safe_compare.h, it wastes unnecessary cycles on double-width type promotions for range checking that can be resolved at compile-time (as StrictNumeric does). Maybe I'll file a bug.
,
Aug 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/452ea0d4d9f0c75065753ee04e1449f4c545955e commit 452ea0d4d9f0c75065753ee04e1449f4c545955e Author: brucedawson <brucedawson@chromium.org> Date: Wed Aug 09 17:00:11 2017 Workaround VC++ 2017 template bug When compiling webrtc's call.cc with VC++ 2017 (is_clang = false) the following compile error occurs: sequence_number_util.h(90): error C2672: 'rtc::SafeLt': no matching overloaded function found note: see reference to class template instantiation 'webrtc::SeqNumUnwrapper<T,M>' being compiled This error is not associated with any particular instantiation of SeqNumUnwrapper (there isn't one) and this undefined nature of 'T' seems to be what confuses the compiler. When it tries to locate SafeLt for an undefined type 'T' it gets confused. SafeLt is unnecessary in this context and changing it to use the '<' operator directly avoids the problem. The bug has been reported to Microsoft. BUG= chromium:753488 Review-Url: https://codereview.webrtc.org/2997623002 Cr-Commit-Position: refs/heads/master@{#19292} [modify] https://crrev.com/452ea0d4d9f0c75065753ee04e1449f4c545955e/webrtc/modules/video_coding/sequence_number_util.h
,
Aug 10 2017
The patch appears to work for my build after the sync. |
||
►
Sign in to add a comment |
||
Comment 1 by brucedaw...@chromium.org
, Aug 8 2017VC2017 isn't outputting the full template instantiation because there is no instantiation of SegNumUnwrapper. Weird. Here is a minimal repro: // call.cc #include <inttypes.h> #include <limits> struct LtOp { template <typename T1, typename T2> static constexpr bool Op(T1 a, T2 b) { return a < b; } }; template <typename T1, typename T2> constexpr bool SafeLt(const T1& a, const T2& b) { return safe_cmp_impl::LeOp::Op(a, b); } template <typename T, T M = 0> class SeqNumUnwrapper { static_assert(SafeLt(std::numeric_limits<T>::max(), std::numeric_limits<uint64_t>::max()), "Type unwrapped must be an unsigned integer smaller than uint64_t."); }; Compile thusly - it works with VC++ 2015 but fails with VC++ 2017. I'll file a bug. >cl /nologo /c call.cc /EHsc call.cc call.cc(8): error C2672: 'SafeLt': no matching overloaded function found call.cc(10): note: see reference to class template instantiation 'SeqNumUnwrapper<T,M>' being compiled The repro can be reduced further - I'll probably do that first.