From 6b0d29efd104c5796fb2f1e84bc39f49acde91bf Mon Sep 17 00:00:00 2001
From: brettw <brettw@chromium.org>
Date: Fri, 23 Sep 2016 09:33:12 -0700
Subject: [PATCH] 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.

Review-Url: https://codereview.chromium.org/2363023002
Cr-Commit-Position: refs/heads/master@{#420626}
---
 base/containers/small_map.h | 12 ++++--------
 base/time/time.h            |  2 --
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/base/containers/small_map.h b/base/containers/small_map.h
index ed96caf1381c6c..2945d58769f28c 100644
--- a/base/containers/small_map.h
+++ b/base/containers/small_map.h
@@ -575,17 +575,13 @@ class SmallMap {
 
   // We want to call constructors and destructors manually, but we don't
   // want to allocate and deallocate the memory used for them separately.
-  // So, we use this crazy ManualConstructor class.
+  // So, we use this crazy ManualConstructor class. Since C++11 it's possible
+  // to use objects in unions like this, but the ManualDestructor syntax is
+  // a bit better and doesn't have limitations on object type.
   //
   // Since array_ and map_ are mutually exclusive, we'll put them in a
-  // union, too.  We add in a dummy_ value which quiets MSVC from otherwise
-  // giving an erroneous "union member has copy constructor" error message
-  // (C2621). This dummy member has to come before array_ to quiet the
-  // compiler.
-  //
-  // TODO(brettw) remove this and use C++11 unions when we require C++11.
+  // union.
   union {
-    ManualConstructor<value_type> dummy_;
     ManualConstructor<value_type> array_[kArraySize];
     ManualConstructor<NormalMap> map_;
   };
diff --git a/base/time/time.h b/base/time/time.h
index b09cd9ef8d09e5..648445f35a2394 100644
--- a/base/time/time.h
+++ b/base/time/time.h
@@ -457,8 +457,6 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
   static Time NowFromSystemTime();
 
   // Converts to/from time_t in UTC and a Time class.
-  // TODO(brettw) this should be removed once everybody starts using the |Time|
-  // class.
   static Time FromTimeT(time_t tt);
   time_t ToTimeT() const;