Bug 1255632 - Make Maybe::map and Maybe::apply support lambdas. r=waldo,njn
☠☠ backed out by 185065fdef03 ☠ ☠
authorSeth Fowler <mark.seth.fowler@gmail.com>
Tue, 21 Jun 2016 20:39:46 -0700
changeset 302290 7b03f23fdde51ce59d834b1e7189121f273b2f19
parent 302289 0b088b6f83128a841076379faceb11416b4006ce
child 302291 15b2940c713102b67cf546308d5d3dd4a34a3bdd
push id78679
push usermfowler@mozilla.com
push dateWed, 22 Jun 2016 05:15:36 +0000
treeherdermozilla-inbound@7b03f23fdde5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswaldo, njn
bugs1255632
milestone50.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 1255632 - Make Maybe::map and Maybe::apply support lambdas. r=waldo,njn
image/ClippedImage.cpp
image/OrientedImage.cpp
mfbt/Maybe.h
mfbt/tests/TestMaybe.cpp
--- a/image/ClippedImage.cpp
+++ b/image/ClippedImage.cpp
@@ -400,34 +400,16 @@ ClippedImage::Draw(gfxContext* aContext,
 
     return result;
   }
 
   return DrawSingleTile(aContext, aSize, aRegion, aWhichFrame,
                         aSamplingFilter, aSVGContext, aFlags);
 }
 
-static SVGImageContext
-UnclipViewport(const SVGImageContext& aOldContext,
-               const pair<nsIntSize, nsIntSize>& aInnerAndClipSize)
-{
-  nsIntSize innerSize(aInnerAndClipSize.first);
-  nsIntSize clipSize(aInnerAndClipSize.second);
-
-  // Map the viewport to the inner image. (Note that we don't take the aSize
-  // parameter of Draw into account, just the clipping region.)
-  CSSIntSize vSize(aOldContext.GetViewportSize());
-  vSize.width = ceil(vSize.width * double(innerSize.width) / clipSize.width);
-  vSize.height =
-    ceil(vSize.height * double(innerSize.height) / clipSize.height);
-
-  return SVGImageContext(vSize,
-                         aOldContext.GetPreserveAspectRatio());
-}
-
 DrawResult
 ClippedImage::DrawSingleTile(gfxContext* aContext,
                              const nsIntSize& aSize,
                              const ImageRegion& aRegion,
                              uint32_t aWhichFrame,
                              SamplingFilter aSamplingFilter,
                              const Maybe<SVGImageContext>& aSVGContext,
                              uint32_t aFlags)
@@ -463,20 +445,34 @@ ClippedImage::DrawSingleTile(gfxContext*
   // the clipping region is placed at the position the caller expects.
   ImageRegion region(aRegion);
   region.MoveBy(clip.x, clip.y);
   region = region.Intersect(clip);
 
   gfxContextMatrixAutoSaveRestore saveMatrix(aContext);
   aContext->Multiply(gfxMatrix::Translation(-clip.x, -clip.y));
 
+  auto unclipViewport = [&](const SVGImageContext& aOldContext) {
+    // Map the viewport to the inner image. Note that we don't take the aSize
+    // parameter of imgIContainer::Draw into account, just the clipping region.
+    // The size in pixels at which the output will ultimately be drawn is
+    // irrelevant here since the purpose of the SVG viewport size is to
+    // determine what *region* of the SVG document will be drawn.
+    CSSIntSize vSize(aOldContext.GetViewportSize());
+    vSize.width = ceil(vSize.width * double(innerSize.width) / mClip.width);
+    vSize.height =
+      ceil(vSize.height * double(innerSize.height) / mClip.height);
+
+    return SVGImageContext(vSize,
+                           aOldContext.GetPreserveAspectRatio());
+  };
+
   return InnerImage()->Draw(aContext, size, region,
                             aWhichFrame, aSamplingFilter,
-                            aSVGContext.map(UnclipViewport,
-                                            make_pair(innerSize, mClip.Size())),
+                            aSVGContext.map(unclipViewport),
                             aFlags);
 }
 
 NS_IMETHODIMP
 ClippedImage::RequestDiscard()
 {
   // We're very aggressive about discarding.
   mCachedSurface = nullptr;
--- a/image/OrientedImage.cpp
+++ b/image/OrientedImage.cpp
@@ -251,28 +251,16 @@ OrientedImage::OrientationMatrix(const n
       break;
     default:
       MOZ_ASSERT(false, "Invalid rotation value");
   }
 
   return builder.Build();
 }
 
-static SVGImageContext
-OrientViewport(const SVGImageContext& aOldContext,
-               const Orientation& aOrientation)
-{
-  CSSIntSize viewportSize(aOldContext.GetViewportSize());
-  if (aOrientation.SwapsWidthAndHeight()) {
-    swap(viewportSize.width, viewportSize.height);
-  }
-  return SVGImageContext(viewportSize,
-                         aOldContext.GetPreserveAspectRatio());
-}
-
 NS_IMETHODIMP_(DrawResult)
 OrientedImage::Draw(gfxContext* aContext,
                     const nsIntSize& aSize,
                     const ImageRegion& aRegion,
                     uint32_t aWhichFrame,
                     SamplingFilter aSamplingFilter,
                     const Maybe<SVGImageContext>& aSVGContext,
                     uint32_t aFlags)
@@ -298,19 +286,27 @@ OrientedImage::Draw(gfxContext* aContext
 
   // The region is already in the orientation expected by the caller, but we
   // need it to be in the image's coordinate system, so we transform it using
   // the inverse of the orientation matrix.
   gfxMatrix inverseMatrix(OrientationMatrix(size, /* aInvert = */ true));
   ImageRegion region(aRegion);
   region.TransformBoundsBy(inverseMatrix);
 
+  auto orientViewport = [&](const SVGImageContext& aOldContext) {
+    CSSIntSize viewportSize(aOldContext.GetViewportSize());
+    if (mOrientation.SwapsWidthAndHeight()) {
+      swap(viewportSize.width, viewportSize.height);
+    }
+    return SVGImageContext(viewportSize,
+                           aOldContext.GetPreserveAspectRatio());
+  };
+
   return InnerImage()->Draw(aContext, size, region, aWhichFrame, aSamplingFilter,
-                            aSVGContext.map(OrientViewport, mOrientation),
-                            aFlags);
+                            aSVGContext.map(orientViewport), aFlags);
 }
 
 nsIntSize
 OrientedImage::OptimalImageSizeForDest(const gfxSize& aDest,
                                        uint32_t aWhichFrame,
                                        SamplingFilter aSamplingFilter,
                                        uint32_t aFlags)
 {
--- a/mfbt/Maybe.h
+++ b/mfbt/Maybe.h
@@ -319,56 +319,60 @@ public:
   const T& operator*() const
   {
     MOZ_ASSERT(mIsSome);
     return ref();
   }
 
   /* If |isSome()|, runs the provided function or functor on the contents of
    * this Maybe. */
-  template<typename F, typename... Args>
-  void apply(F&& aFunc, Args&&... aArgs)
+  template<typename Func>
+  Maybe& apply(Func aFunc)
   {
     if (isSome()) {
-      aFunc(ref(), Forward<Args>(aArgs)...);
+      aFunc(ref());
     }
+    return *this;
   }
 
-  template<typename F, typename... Args>
-  void apply(F&& aFunc, Args&&... aArgs) const
+  template<typename Func>
+  const Maybe& apply(Func aFunc) const
   {
     if (isSome()) {
-      aFunc(ref(), Forward<Args>(aArgs)...);
+      aFunc(ref());
     }
+    return *this;
   }
 
   /*
    * If |isSome()|, runs the provided function and returns the result wrapped
    * in a Maybe. If |isNothing()|, returns an empty Maybe value.
    */
-  template<typename R, typename... FArgs, typename... Args>
-  Maybe<R> map(R (*aFunc)(T&, FArgs...), Args&&... aArgs)
+  template<typename Func>
+  auto map(Func aFunc) -> Maybe<decltype(aFunc(ref()))>
   {
+    using ReturnType = decltype(aFunc(ref()));
     if (isSome()) {
-      Maybe<R> val;
-      val.emplace(aFunc(ref(), Forward<Args>(aArgs)...));
+      Maybe<ReturnType> val;
+      val.emplace(aFunc(ref()));
       return val;
     }
-    return Maybe<R>();
+    return Maybe<ReturnType>();
   }
 
-  template<typename R, typename... FArgs, typename... Args>
-  Maybe<R> map(R (*aFunc)(const T&, FArgs...), Args&&... aArgs) const
+  template<typename Func>
+  auto map(Func aFunc) const -> Maybe<decltype(aFunc(ref()))>
   {
+    using ReturnType = decltype(aFunc(ref()));
     if (isSome()) {
-      Maybe<R> val;
-      val.emplace(aFunc(ref(), Forward<Args>(aArgs)...));
+      Maybe<ReturnType> val;
+      val.emplace(aFunc(ref()));
       return val;
     }
-    return Maybe<R>();
+    return Maybe<ReturnType>();
   }
 
   /* If |isSome()|, empties this Maybe and destroys its contents. */
   void reset()
   {
     if (isSome()) {
       ref().~T();
       mIsSome = false;
--- a/mfbt/tests/TestMaybe.cpp
+++ b/mfbt/tests/TestMaybe.cpp
@@ -522,112 +522,76 @@ static bool gFunctionWasApplied = false;
 static void
 IncrementTag(BasicValue& aValue)
 {
   gFunctionWasApplied = true;
   aValue.SetTag(aValue.GetTag() + 1);
 }
 
 static void
-IncrementTagBy(BasicValue& aValue, int aAmount)
-{
-  gFunctionWasApplied = true;
-  aValue.SetTag(aValue.GetTag() + aAmount);
-}
-
-static void
 AccessValue(const BasicValue&)
 {
   gFunctionWasApplied = true;
 }
 
-static void
-AccessValueWithArg(const BasicValue&, int)
-{
-  gFunctionWasApplied = true;
-}
-
 struct IncrementTagFunctor
 {
-  IncrementTagFunctor() : mBy(1), mArgMoved(false) { }
+  IncrementTagFunctor() : mBy(1) { }
 
   void operator()(BasicValue& aValue)
   {
     aValue.SetTag(aValue.GetTag() + mBy.GetTag());
   }
 
-  void operator()(BasicValue& aValue, const BasicValue& aArg)
-  {
-    mArgMoved = false;
-    aValue.SetTag(aValue.GetTag() + aArg.GetTag());
-  }
-
-  void operator()(BasicValue& aValue, BasicValue&& aArg)
-  {
-    mArgMoved = true;
-    aValue.SetTag(aValue.GetTag() + aArg.GetTag());
-  }
-
   BasicValue mBy;
-  bool mArgMoved;
 };
 
 static bool
 TestApply()
 {
   // Check that apply handles the 'Nothing' case.
   gFunctionWasApplied = false;
   Maybe<BasicValue> mayValue;
   mayValue.apply(&IncrementTag);
   mayValue.apply(&AccessValue);
-  mayValue.apply(&IncrementTagBy, 1);
-  mayValue.apply(&AccessValueWithArg, 1);
   MOZ_RELEASE_ASSERT(!gFunctionWasApplied);
 
   // Check that apply handles the 'Some' case.
   mayValue = Some(BasicValue(1));
   mayValue.apply(&IncrementTag);
   MOZ_RELEASE_ASSERT(gFunctionWasApplied);
   MOZ_RELEASE_ASSERT(mayValue->GetTag() == 2);
   gFunctionWasApplied = false;
   mayValue.apply(&AccessValue);
   MOZ_RELEASE_ASSERT(gFunctionWasApplied);
-  gFunctionWasApplied = false;
-  mayValue.apply(&IncrementTagBy, 2);
-  MOZ_RELEASE_ASSERT(gFunctionWasApplied);
-  MOZ_RELEASE_ASSERT(mayValue->GetTag() == 4);
-  gFunctionWasApplied = false;
-  mayValue.apply(&AccessValueWithArg, 1);
-  MOZ_RELEASE_ASSERT(gFunctionWasApplied);
 
   // Check that apply works with a const reference.
   const Maybe<BasicValue>& mayValueCRef = mayValue;
   gFunctionWasApplied = false;
   mayValueCRef.apply(&AccessValue);
   MOZ_RELEASE_ASSERT(gFunctionWasApplied);
-  gFunctionWasApplied = false;
-  mayValueCRef.apply(&AccessValueWithArg, 1);
-  MOZ_RELEASE_ASSERT(gFunctionWasApplied);
 
   // Check that apply works with functors.
   IncrementTagFunctor tagIncrementer;
   MOZ_RELEASE_ASSERT(tagIncrementer.mBy.GetStatus() == eWasConstructed);
   mayValue = Some(BasicValue(1));
   mayValue.apply(tagIncrementer);
   MOZ_RELEASE_ASSERT(mayValue->GetTag() == 2);
   MOZ_RELEASE_ASSERT(tagIncrementer.mBy.GetStatus() == eWasConstructed);
-  mayValue.apply(tagIncrementer, BasicValue(2));
+
+  // Check that apply works with lambda expressions.
+  int32_t two = 2;
+  gFunctionWasApplied = false;
+  mayValue = Some(BasicValue(2));
+  mayValue.apply([&](BasicValue& aVal) { aVal.SetTag(aVal.GetTag() * two); });
   MOZ_RELEASE_ASSERT(mayValue->GetTag() == 4);
-  MOZ_RELEASE_ASSERT(tagIncrementer.mBy.GetStatus() == eWasConstructed);
-  MOZ_RELEASE_ASSERT(tagIncrementer.mArgMoved == true);
-  BasicValue incrementBy(3);
-  mayValue.apply(tagIncrementer, incrementBy);
-  MOZ_RELEASE_ASSERT(mayValue->GetTag() == 7);
-  MOZ_RELEASE_ASSERT(tagIncrementer.mBy.GetStatus() == eWasConstructed);
-  MOZ_RELEASE_ASSERT(tagIncrementer.mArgMoved == false);
+  mayValue.apply([=](BasicValue& aVal) { aVal.SetTag(aVal.GetTag() * two); });
+  MOZ_RELEASE_ASSERT(mayValue->GetTag() == 8);
+  mayValueCRef.apply([&](const BasicValue& aVal) { gFunctionWasApplied = true; });
+  MOZ_RELEASE_ASSERT(gFunctionWasApplied == true);
 
   return true;
 }
 
 static int
 TimesTwo(const BasicValue& aValue)
 {
   return aValue.GetTag() * 2;
@@ -636,109 +600,78 @@ TimesTwo(const BasicValue& aValue)
 static int
 TimesTwoAndResetOriginal(BasicValue& aValue)
 {
   int tag = aValue.GetTag();
   aValue.SetTag(1);
   return tag * 2;
 }
 
-static int
-TimesNum(const BasicValue& aValue, int aNum)
-{
-  return aValue.GetTag() * aNum;
-}
-
-static int
-TimesNumAndResetOriginal(BasicValue& aValue, int aNum)
-{
-  int tag = aValue.GetTag();
-  aValue.SetTag(1);
-  return tag * aNum;
-}
-
 struct MultiplyTagFunctor
 {
-  MultiplyTagFunctor() : mBy(2), mArgMoved(false) { }
+  MultiplyTagFunctor() : mBy(2) { }
 
   int operator()(BasicValue& aValue)
   {
     return aValue.GetTag() * mBy.GetTag();
   }
 
-  int operator()(BasicValue& aValue, const BasicValue& aArg)
-  {
-    mArgMoved = false;
-    return aValue.GetTag() * aArg.GetTag();
-  }
-
-  int operator()(BasicValue& aValue, BasicValue&& aArg)
-  {
-    mArgMoved = true;
-    return aValue.GetTag() * aArg.GetTag();
-  }
-
   BasicValue mBy;
-  bool mArgMoved;
 };
 
 static bool
 TestMap()
 {
   // Check that map handles the 'Nothing' case.
   Maybe<BasicValue> mayValue;
   MOZ_RELEASE_ASSERT(mayValue.map(&TimesTwo) == Nothing());
   static_assert(IsSame<Maybe<int>,
                        DECLTYPE(mayValue.map(&TimesTwo))>::value,
                 "map(TimesTwo) should return a Maybe<int>");
   MOZ_RELEASE_ASSERT(mayValue.map(&TimesTwoAndResetOriginal) == Nothing());
-  MOZ_RELEASE_ASSERT(mayValue.map(&TimesNum, 3) == Nothing());
-  static_assert(IsSame<Maybe<int>,
-                       DECLTYPE(mayValue.map(&TimesNum, 3))>::value,
-                "map(TimesNum, 3) should return a Maybe<int>");
-  MOZ_RELEASE_ASSERT(mayValue.map(&TimesNumAndResetOriginal, 3) == Nothing());
 
   // Check that map handles the 'Some' case.
   mayValue = Some(BasicValue(2));
   MOZ_RELEASE_ASSERT(mayValue.map(&TimesTwo) == Some(4));
   MOZ_RELEASE_ASSERT(mayValue.map(&TimesTwoAndResetOriginal) == Some(4));
   MOZ_RELEASE_ASSERT(mayValue->GetTag() == 1);
   mayValue = Some(BasicValue(2));
-  MOZ_RELEASE_ASSERT(mayValue.map(&TimesNum, 3) == Some(6));
-  MOZ_RELEASE_ASSERT(mayValue.map(&TimesNumAndResetOriginal, 3) == Some(6));
-  MOZ_RELEASE_ASSERT(mayValue->GetTag() == 1);
 
   // Check that map works with a const reference.
   mayValue->SetTag(2);
   const Maybe<BasicValue>& mayValueCRef = mayValue;
   MOZ_RELEASE_ASSERT(mayValueCRef.map(&TimesTwo) == Some(4));
   static_assert(IsSame<Maybe<int>,
                        DECLTYPE(mayValueCRef.map(&TimesTwo))>::value,
                 "map(TimesTwo) should return a Maybe<int>");
-  MOZ_RELEASE_ASSERT(mayValueCRef.map(&TimesNum, 3) == Some(6));
-  static_assert(IsSame<Maybe<int>,
-                       DECLTYPE(mayValueCRef.map(&TimesNum, 3))>::value,
-                "map(TimesNum, 3) should return a Maybe<int>");
 
   // Check that map works with functors.
-  // XXX(seth): Support for functors will be added in bug 1054115; it had to be
-  // ripped out temporarily because of incompatibilities with GCC 4.4.
-  /*
   MultiplyTagFunctor tagMultiplier;
   MOZ_RELEASE_ASSERT(tagMultiplier.mBy.GetStatus() == eWasConstructed);
   MOZ_RELEASE_ASSERT(mayValue.map(tagMultiplier) == Some(4));
   MOZ_RELEASE_ASSERT(tagMultiplier.mBy.GetStatus() == eWasConstructed);
-  MOZ_RELEASE_ASSERT(mayValue.map(tagMultiplier, BasicValue(3)) == Some(6));
-  MOZ_RELEASE_ASSERT(tagMultiplier.mBy.GetStatus() == eWasConstructed);
-  MOZ_RELEASE_ASSERT(tagMultiplier.mArgMoved == true);
-  BasicValue multiplyBy(3);
-  MOZ_RELEASE_ASSERT(mayValue.map(tagMultiplier, multiplyBy) == Some(6));
-  MOZ_RELEASE_ASSERT(tagMultiplier.mBy.GetStatus() == eWasConstructed);
-  MOZ_RELEASE_ASSERT(tagMultiplier.mArgMoved == false);
-  */
+
+  // Check that map works with lambda expressions.
+  int two = 2;
+  mayValue = Some(BasicValue(2));
+  Maybe<int> mappedValue =
+    mayValue.map([&](const BasicValue& aVal) {
+      return aVal.GetTag() * two;
+    });
+  MOZ_RELEASE_ASSERT(mappedValue == Some(4));
+  mappedValue =
+    mayValue.map([=](const BasicValue& aVal) {
+      return aVal.GetTag() * two;
+    });
+  MOZ_RELEASE_ASSERT(mappedValue == Some(4));
+  mappedValue =
+    mayValueCRef.map([&](const BasicValue& aVal) {
+      return aVal.GetTag() * two;
+    });
+  MOZ_RELEASE_ASSERT(mappedValue == Some(4));
 
   return true;
 }
 
 static bool
 TestToMaybe()
 {
   BasicValue value(1);