Bug 1356843 - Fix -Wcomma warnings in layout/. r?dholbert draft
authorChris Peterson <cpeterson@mozilla.com>
Sun, 09 Apr 2017 21:11:45 -0700
changeset 563347 528afe0a605c341a79d98ed2396a058ab1536e1e
parent 563346 6285a5d92c1c0a0f8d0249945cd5c81d9c105ec7
child 563348 007462b7dc00e188382982ddf4a03f0c43be83e3
push id54258
push usercpeterson@mozilla.com
push dateSun, 16 Apr 2017 05:52:14 +0000
reviewersdholbert
bugs1356843
milestone55.0a1
Bug 1356843 - Fix -Wcomma warnings in layout/. r?dholbert clang's -Wcomma warning warns about suspicious use of the comma operator such as between two statements or to call a function for side effects within an expression. In addition to fixing the warnings, replace some char16_t(' ') casts of char literals with shorter UTF-16 character literals u' '. layout/painting/FrameLayerBuilder.cpp:3647:31 [-Wcomma] possible misuse of comma operator here layout/painting/FrameLayerBuilder.cpp:3657:41 [-Wcomma] possible misuse of comma operator here layout/painting/nsCSSRenderingBorders.cpp:3336:33 [-Wcomma] possible misuse of comma operator here layout/painting/nsCSSRenderingBorders.cpp:3336:60 [-Wcomma] possible misuse of comma operator here layout/painting/nsCSSRenderingBorders.cpp:3337:33 [-Wcomma] possible misuse of comma operator here layout/painting/nsCSSRenderingBorders.cpp:3337:60 [-Wcomma] possible misuse of comma operator here layout/style/Declaration.cpp:808:41 [-Wcomma] possible misuse of comma operator here layout/style/Declaration.cpp:812:42 [-Wcomma] possible misuse of comma operator here layout/style/FontFaceSet.cpp:1118:60 [-Wcomma] possible misuse of comma operator here MozReview-Commit-ID: 9tfcIsnnBwM
layout/painting/FrameLayerBuilder.cpp
layout/painting/nsCSSRenderingBorders.cpp
layout/style/Declaration.cpp
layout/style/FontFaceSet.cpp
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -3655,27 +3655,27 @@ ContainerState::NewPaintedLayerData(nsDi
                                     const ActiveScrolledRoot* aASR,
                                     const DisplayItemClipChain* aClipChain,
                                     const ActiveScrolledRoot* aScrollMetadataASR,
                                     const nsPoint& aTopLeft)
 {
   PaintedLayerData data;
   data.mAnimatedGeometryRoot = aAnimatedGeometryRoot;
   data.mASR = aASR;
-  data.mClipChain = aClipChain,
+  data.mClipChain = aClipChain;
   data.mAnimatedGeometryRootOffset = aTopLeft;
   data.mReferenceFrame = aItem->ReferenceFrame();
   data.mBackfaceHidden = aItem->Frame()->In3DContextAndBackfaceIsHidden();
 
   data.mNewChildLayersIndex = mNewChildLayers.Length();
   NewLayerEntry* newLayerEntry = mNewChildLayers.AppendElement();
   newLayerEntry->mAnimatedGeometryRoot = aAnimatedGeometryRoot;
   newLayerEntry->mASR = aASR;
   newLayerEntry->mScrollMetadataASR = aScrollMetadataASR;
-  newLayerEntry->mClipChain = aClipChain,
+  newLayerEntry->mClipChain = aClipChain;
   // newLayerEntry->mOpaqueRegion is filled in later from
   // paintedLayerData->mOpaqueRegion, if necessary.
 
   // Allocate another entry for this layer's optimization to ColorLayer/ImageLayer
   mNewChildLayers.AppendElement();
 
   return data;
 }
--- a/layout/painting/nsCSSRenderingBorders.cpp
+++ b/layout/painting/nsCSSRenderingBorders.cpp
@@ -3328,18 +3328,18 @@ nsCSSBorderRenderer::DrawBorders()
   }
 
   // If we have composite colors -and- border radius,
   // then use separate corners so we get OP_ADD for the corners.
   // Otherwise, we'll get artifacts as we draw stacked 1px-wide curves.
   if (allBordersSame && mCompositeColors[0] != nullptr && !mNoBorderRadius)
     forceSeparateCorners = true;
 
-  PrintAsString(" mOuterRect: "), PrintAsString(mOuterRect), PrintAsStringNewline();
-  PrintAsString(" mInnerRect: "), PrintAsString(mInnerRect), PrintAsStringNewline();
+  PrintAsString(" mOuterRect: "); PrintAsString(mOuterRect); PrintAsStringNewline();
+  PrintAsString(" mInnerRect: "); PrintAsString(mInnerRect); PrintAsStringNewline();
   PrintAsFormatString(" mBorderColors: 0x%08x 0x%08x 0x%08x 0x%08x\n", mBorderColors[0], mBorderColors[1], mBorderColors[2], mBorderColors[3]);
 
   // if conditioning the outside rect failed, then bail -- the outside
   // rect is supposed to enclose the entire border
   {
     gfxRect outerRect = ThebesRect(mOuterRect);
     outerRect.Condition();
     if (outerRect.IsEmpty())
--- a/layout/style/Declaration.cpp
+++ b/layout/style/Declaration.cpp
@@ -795,27 +795,36 @@ Declaration::GetPropertyValueInternal(
     case eCSSProperty_border_block_end:
     case eCSSProperty_column_rule:
     case eCSSProperty_outline: {
       const nsCSSPropertyID* subprops =
         nsCSSProps::SubpropertyEntryFor(aProperty);
       MOZ_ASSERT(StringEndsWith(nsCSSProps::GetStringValue(subprops[2]),
                                 NS_LITERAL_CSTRING("-color")),
                  "third subprop must be the color property");
-      const nsCSSValue *colorValue = data->ValueFor(subprops[2]);
-      bool isCurrentColor =
-        colorValue->GetUnit() == eCSSUnit_EnumColor &&
-        colorValue->GetIntValue() == NS_COLOR_CURRENTCOLOR;
-      if (!AppendValueToString(subprops[0], aValue, aSerialization) ||
-          !(aValue.Append(char16_t(' ')),
-            AppendValueToString(subprops[1], aValue, aSerialization)) ||
+
+      bool ok = AppendValueToString(subprops[0], aValue, aSerialization);
+      if (ok) {
+        aValue.Append(u' ');
+        ok = AppendValueToString(subprops[1], aValue, aSerialization);
+        if (ok) {
+          const nsCSSValue *colorValue = data->ValueFor(subprops[2]);
+          bool isCurrentColor =
+            colorValue->GetUnit() == eCSSUnit_EnumColor &&
+            colorValue->GetIntValue() == NS_COLOR_CURRENTCOLOR;
+
           // Don't output a third value when it's currentcolor.
-          !(isCurrentColor ||
-            (aValue.Append(char16_t(' ')),
-             AppendValueToString(subprops[2], aValue, aSerialization)))) {
+          if (!isCurrentColor) {
+            aValue.Append(u' ');
+            ok = AppendValueToString(subprops[2], aValue, aSerialization);
+          }
+        }
+      }
+
+      if (!ok) {
         aValue.Truncate();
       }
       break;
     }
     case eCSSProperty_background: {
       GetImageLayerValue(data, aValue, aSerialization,
                          nsStyleImageLayers::kBackgroundLayerTable);
       break;
--- a/layout/style/FontFaceSet.cpp
+++ b/layout/style/FontFaceSet.cpp
@@ -1111,18 +1111,22 @@ FontFaceSet::FindOrCreateUserFontEntryFr
           // the same-site origin check and access control headers are
           // enforced against the sheet principal rather than the document
           // principal to allow user stylesheets to include @font-face rules
           face->mUseOriginPrincipal = (aSheetType == SheetType::User ||
                                        aSheetType == SheetType::Agent);
 
           face->mLocalName.Truncate();
           face->mFormatFlags = 0;
-          while (i + 1 < numSrc && (val = srcArr->Item(i+1),
-                   val.GetUnit() == eCSSUnit_Font_Format)) {
+
+          while (i + 1 < numSrc) {
+            val = srcArr->Item(i+1);
+            if (val.GetUnit() != eCSSUnit_Font_Format)
+              break;
+
             nsDependentString valueString(val.GetStringBufferValue());
             if (valueString.LowerCaseEqualsASCII("woff")) {
               face->mFormatFlags |= gfxUserFontSet::FLAG_FORMAT_WOFF;
             } else if (Preferences::GetBool(GFX_PREF_WOFF2_ENABLED) &&
                        valueString.LowerCaseEqualsASCII("woff2")) {
               face->mFormatFlags |= gfxUserFontSet::FLAG_FORMAT_WOFF2;
             } else if (valueString.LowerCaseEqualsASCII("opentype")) {
               face->mFormatFlags |= gfxUserFontSet::FLAG_FORMAT_OPENTYPE;