This feels like a bug to me, but 
a) It's not documented, so it's hard to say what our intention is.
b) I wonder how many people are using this.

I suspect we should err on the side of caution and deprecate-and-add somehow. I guess that union() has the same problem.

Yes, I'm guessing that there are very few apps that are (intentionally) using this behavior, but deprecate-and-addition is probably the safest option

It does not seem a bug to me at all. It's common practice in C++ to have non-const member functions modifying an object and returning a reference to the modified objects. This allows you to do additional calls to similar member functions on the same object. Think of all the operator+=, operator-= and the like. The typical pattern for such a member function is something like:

SomeClass &SomeClass::some_method (const SomeClass &other)
{
  // modify *this depending on other
  ...
  return *this;
}

The solution provided above would not work, because it returns a reference to the local instance "intersection" which will be destroyed when the function returns. The caller will not be able to savely use the return value of the function.

A function behaving the way you want it to should have a signature like:

Rectangle Rectangle::intersect(const Rectangle& src2) const

That means: It should be const, because it does not modify the object and it has to return a newly created Rectangle by value, because there is no surviving Rectangle which could be returned by reference.

You're correct that my example code above would not compile because I accidentally left the return type as a reference and forgot to add const.  I was not attempting to provide working code, i was simply trying to illustrate an alternate API.

In addition, the point of this bug report was not really about whether the current implementation is *wrong* (I know there are valid reasons for returning a reference to a modified object in some situations), the point of the bug report was about the fact that you *cannot* calculate an intersection without modifying the calling object with the current API, and that this makes certain things quite awkward.  If you want to calculate an intersection of two rectangles without changing the original rectangles, you need to do manual copies like:

Gdk::Rectangle A(...);
Gdk::Rectangle B(...);
// now I want to calculate the intersection of A and B without 
// changing either A or B, so I need to make a copy
Gdk::Rectangle C(A);
C.intersect (B); // now C is the intersection of A and B

That seems remarkably awkward for a simple use case.  In addition, there is currently no way to calculate an intersection of two const rectangles.

Sorry. I missunderstood your intention. I thought you wanted to change the code without changing the return type (I think it would compile but it would crash at runtime).
Of course your right, that there should be a way to do an intersection without modifying the operands.

I'd vote for a freestanding intersect() function in the Gdk namespace. Methods are always a bit awkward for commutative operators.

By the way, I'm responsible for the current interface. It's in line with the common STL practice of having append(), insert() etc. methods modify the object.

yes, a freestanding function would feel much more comfortable to me.  We could still leave the intersect() member function as well -- there's nothing *wrong* with it per se, it just doesn't provide the flexibility i'd like.  it could still be useful in certain circumstances.

Feel free to make that change in gtkmm 3 (git master) though time is running out.

I have at least now documented the existing methods.

Wouldn't it be possible just to add another member function that doesn't modify *this? This would both solve the problem and be backwards-compatible. For the name of the new function, I'd suggest 'intersected'. This name is very intuitive,
 since the distinction with 'intersect' is accentuated. As a bonus, Qt uses the same name with the same behaviour (returns new instance) in the API of QRect.

We could also add 'joined' function to solve similar problem with 'join'. Also, I think it would be a good idea to add another member function 'intersects' for checking whether two rectangles intersect. The current way to do this is a bit awkward. This function also has an equivalent in the QRect API.

    Mark

Created attachment 220125 [details] [review]
Rectangle: add intersected() and joined() methods

The attached patch adds non-modifying intersected() and joined() methods.

Have you considered the suggestion in comment 6 and comment 7, i.e. free-
standing functions instead of member functions? Something like

namespace Gdk
{
Rectangle intersect(const Rectangle& src1, const Rectangle& src2)
{
  Rectangle dest;
  gdk_rectangle_intersect(&src1.gobject_, &src2.gobject_, &dest.gobject_);
  return dest;
}
} // namespace Gdk

and similarly for join() and a 3-parameter intersect() with
bool& rectangles_intersect.

Created attachment 232408 [details] [review]
patch: Gdk::Rectangle: Add global join() and intersect() functions.

Here's an alternative to the patch in comment 11.
Which one shall I push? Anyone still interested in this matter?

I've pushed the patch in comment 13, but with another title:
Gdk::Rectangle: Add Gdk::join() and Gdk::intersect() nonmember functions.