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

Issue 11819038: re2 building on x64 windows (Closed)

Created:
7 years, 11 months ago by scottmg
Modified:
7 years, 11 months ago
Reviewers:
jschuh
CC:
chromium-reviews
Visibility:
Public.

Description

re2 building on x64 windows Mostly disable c4267, but a couple that seem safe to me. R=jschuh@chromium.org BUG=167187 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175978

Patch Set 1 #

Patch Set 2 : fix warnings in re2 for win x64 compilation #

Patch Set 3 : just a couple minor non-sign-changing fixes, and disable c4267 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -3 lines) Patch
M third_party/re2/README.chromium View 1 1 chunk +1 line, -0 lines 0 comments Download
A third_party/re2/patches/msvc-x64.patch View 1 2 1 chunk +31 lines, -0 lines 0 comments Download
M third_party/re2/re2.gyp View 2 1 chunk +1 line, -1 line 0 comments Download
M third_party/re2/re2/dfa.cc View 1 2 1 chunk +2 lines, -2 lines 1 comment Download
M third_party/re2/util/logging.h View 1 1 chunk +4 lines, -0 lines 1 comment Download

Messages

Total messages: 6 (0 generated)
scottmg
7 years, 11 months ago (2013-01-09 21:41:12 UTC) #1
jschuh
On 2013/01/09 21:41:12, scottmg wrote: Did you actually verify all the truncations are safe?
7 years, 11 months ago (2013-01-09 21:59:55 UTC) #2
scottmg
now without a bunch of arbitrary truncations. https://codereview.chromium.org/11819038/diff/5001/third_party/re2/re2/dfa.cc File third_party/re2/re2/dfa.cc (right): https://codereview.chromium.org/11819038/diff/5001/third_party/re2/re2/dfa.cc#newcode475 third_party/re2/re2/dfa.cc:475: if (state_budget_ ...
7 years, 11 months ago (2013-01-09 22:28:34 UTC) #3
jschuh
lgtm
7 years, 11 months ago (2013-01-09 22:34:26 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/scottmg@chromium.org/11819038/5001
7 years, 11 months ago (2013-01-09 22:41:36 UTC) #5
commit-bot: I haz the power
7 years, 11 months ago (2013-01-10 01:54:58 UTC) #6
Message was sent while issue was closed.
Change committed as 175978

Powered by Google App Engine
This is Rietveld 408576698