Bug 1508818, part 3 - Shrink comments so they don't get poorly reflowed by clang-format. r=froydnj
authorAndrew McCreight <continuation@gmail.com>
Wed, 28 Nov 2018 21:06:09 +0000
changeset 507805 67b1d1e433013e79b67684b48c4c074712e2509e
parent 507804 b226d4c40893fdd080f65c9d8d8a52990a5386be
child 507806 09c2de91a87f13c8de2da8508bae2bc2daff6b70
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1508818
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1508818, part 3 - Shrink comments so they don't get poorly reflowed by clang-format. r=froydnj Clang format does not always reflow comments correctly to get them within 80 columns. The major categories of failures I have noticed in xpcom/ are: - Comments that are lists. I fixed these by manually getting them so they'll be within 80 columns after clang-format runs. - Comments intermixed with lists of things like enums, initializers, or even fields in a class. It doesn't seem to associate the comment with the item in the list correctly. The worst cases of these happen when it changes initializer lists from having commas at the start of each item to having them at the end. In the initializer comma cases, I fixed them by making the commas at the end, so clang-format won't mix things up. For other cases, I often moved the comment for an item onto its own line, because it was not possible to have both the comment and the item on the same line and stay within 80 columns. - One odd case is nsEnumeratorUtils.cpp, where the end of line comment after a NS_DECL macro confused clang-format and made it stop realizing that the NS_DECL thing was a complete statement. I also added a blank line to that file before a declaration because I think that is better. Depends on D13183 Differential Revision: https://phabricator.services.mozilla.com/D13184
xpcom/base/CycleCollectedJSContext.h
xpcom/base/nsCycleCollector.cpp
xpcom/base/nsMemoryReporterManager.cpp
xpcom/base/nsTraceRefcnt.cpp
xpcom/ds/nsEnumeratorUtils.cpp
xpcom/string/nsStringFlags.h
xpcom/threads/TimerThread.cpp
--- a/xpcom/base/CycleCollectedJSContext.h
+++ b/xpcom/base/CycleCollectedJSContext.h
@@ -59,17 +59,18 @@ struct CycleCollectorResults
     mFreedJSZones = 0;
     mNumSlices = 1;
     // mNumSlices is initialized to one, because we call Init() after the
     // per-slice increment of mNumSlices has already occurred.
   }
 
   bool mForcedGC;
   bool mMergedZones;
-  bool mAnyManual; // true if any slice of the CC was manually triggered, or at shutdown.
+  // mAnyManual is true if any slice was manually triggered, and at shutdown.
+  bool mAnyManual;
   uint32_t mVisitedRefCounted;
   uint32_t mVisitedGCed;
   uint32_t mFreedRefCounted;
   uint32_t mFreedGCed;
   uint32_t mFreedJSZones;
   uint32_t mNumSlices;
 };
 
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -1154,18 +1154,18 @@ public:
   {
     mCount = 0;
     mEntries.Clear();
   }
 
   void SelectPointers(CCGraphBuilder& aBuilder);
 
   // RemoveSkippable removes entries from the purple buffer synchronously
-  // (1) if aAsyncSnowWhiteFreeing is false and nsPurpleBufferEntry::mRefCnt is 0 or
-  // (2) if the object's nsXPCOMCycleCollectionParticipant::CanSkip() returns true or
+  // (1) if !aAsyncSnowWhiteFreeing and nsPurpleBufferEntry::mRefCnt is 0 or
+  // (2) if nsXPCOMCycleCollectionParticipant::CanSkip() for the obj or
   // (3) if nsPurpleBufferEntry::mRefCnt->IsPurple() is false.
   // (4) If removeChildlessNodes is true, then any nodes in the purple buffer
   //     that will have no children in the cycle collector graph will also be
   //     removed. CanSkip() may be run on these children.
   void RemoveSkippable(nsCycleCollector* aCollector,
                        js::SliceBudget& aBudget,
                        bool aRemoveChildlessNodes,
                        bool aAsyncSnowWhiteFreeing,
@@ -1243,17 +1243,18 @@ enum ccPhase
   IdlePhase,
   GraphBuildingPhase,
   ScanAndCollectWhitePhase,
   CleanupPhase
 };
 
 enum ccType
 {
-  SliceCC,     /* If a CC is in progress, continue it. Otherwise, start a new one. */
+  SliceCC,     /* If a CC is in progress, continue it.
+                  Otherwise, start a new one. */
   ManualCC,    /* Explicitly triggered. */
   ShutdownCC   /* Shutdown CC, used for finding leaks. */
 };
 
 ////////////////////////////////////////////////////////////////////////
 // Top level structure for the cycle collector.
 ////////////////////////////////////////////////////////////////////////
 
--- a/xpcom/base/nsMemoryReporterManager.cpp
+++ b/xpcom/base/nsMemoryReporterManager.cpp
@@ -2238,28 +2238,28 @@ nsMemoryReporterManager::FinishReporting
 nsMemoryReporterManager::PendingProcessesState::PendingProcessesState(
     uint32_t aGeneration, bool aAnonymize, bool aMinimize,
     uint32_t aConcurrencyLimit,
     nsIHandleReportCallback* aHandleReport,
     nsISupports* aHandleReportData,
     nsIFinishReportingCallback* aFinishReporting,
     nsISupports* aFinishReportingData,
     const nsAString& aDMDDumpIdent)
-  : mGeneration(aGeneration)
-  , mAnonymize(aAnonymize)
-  , mMinimize(aMinimize)
-  , mChildrenPending()
-  , mNumProcessesRunning(1) // reporting starts with the parent
-  , mNumProcessesCompleted(0)
-  , mConcurrencyLimit(aConcurrencyLimit)
-  , mHandleReport(aHandleReport)
-  , mHandleReportData(aHandleReportData)
-  , mFinishReporting(aFinishReporting)
-  , mFinishReportingData(aFinishReportingData)
-  , mDMDDumpIdent(aDMDDumpIdent)
+  : mGeneration(aGeneration),
+    mAnonymize(aAnonymize),
+    mMinimize(aMinimize),
+    mChildrenPending(),
+    mNumProcessesRunning(1), // reporting starts with the parent
+    mNumProcessesCompleted(0),
+    mConcurrencyLimit(aConcurrencyLimit),
+    mHandleReport(aHandleReport),
+    mHandleReportData(aHandleReportData),
+    mFinishReporting(aFinishReporting),
+    mFinishReportingData(aFinishReportingData),
+    mDMDDumpIdent(aDMDDumpIdent)
 {
 }
 
 static void
 CrashIfRefcountIsZero(nsISupports* aObj)
 {
   // This will probably crash if the object's refcount is 0.
   uint32_t refcnt = NS_ADDREF(aObj);
--- a/xpcom/base/nsTraceRefcnt.cpp
+++ b/xpcom/base/nsTraceRefcnt.cpp
@@ -346,18 +346,21 @@ public:
               nsCRT::strcmp(mClassName, "TOTAL") ? (mStats.NumLeaked() * GetClassSize()) : mTotalLeaked,
               mStats.mCreates,
               mStats.NumLeaked());
     }
   }
 
 protected:
   char* mClassName;
-  double mClassSize; // This is stored as a double because of the way we compute the avg class size for total bloat.
-  int64_t mTotalLeaked; // Used only for TOTAL entry.
+  // mClassSize is stored as a double because of the way we compute the avg
+  // class size for total bloat.
+  double mClassSize;
+  // mTotalLeaked is only used for the TOTAL entry.
+  int64_t mTotalLeaked;
   nsTraceRefcntStats mStats;
 };
 
 static void
 RecreateBloatView()
 {
   gBloatView = new BloatHash(256);
 }
--- a/xpcom/ds/nsEnumeratorUtils.cpp
+++ b/xpcom/ds/nsEnumeratorUtils.cpp
@@ -17,25 +17,26 @@
 class EmptyEnumeratorImpl
   : public nsSimpleEnumerator
   , public nsIUTF8StringEnumerator
   , public nsIStringEnumerator
 {
 public:
   EmptyEnumeratorImpl() {}
 
-  // nsISupports interface
-  NS_DECL_ISUPPORTS_INHERITED  // not really inherited, but no mRefCnt
+  // nsISupports interface. Not really inherited, but no mRefCnt.
+  NS_DECL_ISUPPORTS_INHERITED
 
   // nsISimpleEnumerator
   NS_DECL_NSISIMPLEENUMERATOR
   NS_DECL_NSIUTF8STRINGENUMERATOR
   NS_DECL_NSISTRINGENUMERATORBASE
   // can't use NS_DECL_NSISTRINGENUMERATOR because they share the
   // HasMore() signature
+
   NS_IMETHOD GetNext(nsAString& aResult) override;
 
   static EmptyEnumeratorImpl* GetInstance()
   {
     static const EmptyEnumeratorImpl kInstance;
     return const_cast<EmptyEnumeratorImpl*>(&kInstance);
   }
 };
--- a/xpcom/string/nsStringFlags.h
+++ b/xpcom/string/nsStringFlags.h
@@ -47,22 +47,33 @@ enum class StringDataFlags : uint16_t
   //   REFCOUNTED, OWNED, or INLINE imply TERMINATED.  This is because
   //   the string classes always allocate null-terminated buffers, and
   //   non-terminated substrings are always dependent.
   //
   //   VOIDED implies TERMINATED, and moreover it implies that mData
   //   points to char_traits::sEmptyBuffer.  Therefore, VOIDED is
   //   mutually exclusive with REFCOUNTED, OWNED, and INLINE.
 
-  TERMINATED   = 1 << 0,  // IsTerminated returns true
-  VOIDED       = 1 << 1,  // IsVoid returns true
-  REFCOUNTED   = 1 << 2,  // mData points to a heap-allocated, shareable, refcounted buffer
-  OWNED        = 1 << 3,  // mData points to a heap-allocated, raw buffer
-  INLINE       = 1 << 4,  // mData points to a writable, inline buffer
-  LITERAL      = 1 << 5   // mData points to a string literal; DataFlags::TERMINATED will also be set
+  // IsTerminated returns true
+  TERMINATED   = 1 << 0,
+
+  // IsVoid returns true
+  VOIDED       = 1 << 1,
+
+  // mData points to a heap-allocated, shareable, refcounted buffer
+  REFCOUNTED   = 1 << 2,
+
+  // mData points to a heap-allocated, raw buffer
+  OWNED        = 1 << 3,
+
+  // mData points to a writable, inline buffer
+  INLINE       = 1 << 4,
+
+  // mData points to a string literal; DataFlags::TERMINATED will also be set
+  LITERAL      = 1 << 5
 };
 
 // bits for mClassFlags
 enum class StringClassFlags : uint16_t
 {
   INLINE          = 1 << 0, // |this|'s buffer is inline
   NULL_TERMINATED = 1 << 1  // |this| requires its buffer is null-terminated
 };
--- a/xpcom/threads/TimerThread.cpp
+++ b/xpcom/threads/TimerThread.cpp
@@ -110,21 +110,21 @@ private:
   };
 
   ArenaAllocator<4096> mPool;
   FreeEntry* mFirstFree;
   mozilla::Monitor mMonitor;
 
 public:
   TimerEventAllocator()
-    : mPool()
-    , mFirstFree(nullptr)
+    : mPool(),
+      mFirstFree(nullptr),
       // Timer thread state may be accessed during GC, so uses of this monitor
       // are not preserved when recording/replaying.
-    , mMonitor("TimerEventAllocator", recordreplay::Behavior::DontPreserve)
+      mMonitor("TimerEventAllocator", recordreplay::Behavior::DontPreserve)
   {
   }
 
   ~TimerEventAllocator()
   {
   }
 
   void* Alloc(size_t aSize);