Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(158)

Issue 2997623002: Workaround VC++ 2017 template bug (Closed)

Created:
3 years, 4 months ago by brucedawson
Modified:
3 years, 4 months ago
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, zhengzhonghou_agora.io, video-team_agora.io, stefan-webrtc, mflodman
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

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} Committed: https://chromium.googlesource.com/external/webrtc/+/452ea0d4d9f0c75065753ee04e1449f4c545955e

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M webrtc/modules/video_coding/sequence_number_util.h View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 18 (11 generated)
brucedawson
Does this look like a reasonable workaround? Thanks.
3 years, 4 months ago (2017-08-08 22:44:39 UTC) #4
sprang_webrtc
Looks ok to me (dont't recall the exact reason we used SafeLt here). philipel@, can ...
3 years, 4 months ago (2017-08-09 08:41:08 UTC) #8
philipel
lgtm
3 years, 4 months ago (2017-08-09 11:17:58 UTC) #11
kwiberg-webrtc
On 2017/08/09 08:41:08, sprang_webrtc wrote: > Looks ok to me (dont't recall the exact reason ...
3 years, 4 months ago (2017-08-09 12:59:27 UTC) #12
sprang_webrtc
lgtm
3 years, 4 months ago (2017-08-09 13:58:38 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2997623002/1
3 years, 4 months ago (2017-08-09 16:57:29 UTC) #15
commit-bot: I haz the power
3 years, 4 months ago (2017-08-09 17:00:18 UTC) #18
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/external/webrtc/+/452ea0d4d9f0c75065753ee04...

Powered by Google App Engine
This is Rietveld 408576698