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

Issue 2363023002: Clean up my TODO comments in SmallMap and Time. (Closed)

Created:
4 years, 3 months ago by brettw
Modified:
4 years, 3 months ago
Reviewers:
Lei Zhang
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Clean up my TODO comments in SmallMap and Time. The TODO in time.h was about removing time_t functions. When I wrote this I was optimistically thinking we wouldn't use time_t any more now that we have the awesome new Time class. This was fantasy so the TODO is removed. SmallMap uses ManualConstructor. The TODO was that these could be removed when we have C++11 unions. This is true, but the syntax looked worse. There was a use in ConvertToRealMap that still required ManualConstructor for optimum allocations, so it seemed best to just keep using ManualConstructor. However, the necessity of having the dummy value in the union due to an MSVC error seems no longer applicable. The dummy value is removed. Committed: https://crrev.com/6b0d29efd104c5796fb2f1e84bc39f49acde91bf Cr-Commit-Position: refs/heads/master@{#420626}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -10 lines) Patch
M base/containers/small_map.h View 1 chunk +4 lines, -8 lines 0 comments Download
M base/time/time.h View 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 12 (7 generated)
brettw
4 years, 3 months ago (2016-09-23 04:38:23 UTC) #4
Lei Zhang
lgtm
4 years, 3 months ago (2016-09-23 04:46:40 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2363023002/1
4 years, 3 months ago (2016-09-23 15:30:58 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 3 months ago (2016-09-23 16:33:31 UTC) #10
commit-bot: I haz the power
4 years, 3 months ago (2016-09-23 16:35:23 UTC) #12
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/6b0d29efd104c5796fb2f1e84bc39f49acde91bf
Cr-Commit-Position: refs/heads/master@{#420626}

Powered by Google App Engine
This is Rietveld 408576698