Created attachment 312115 [details] [review]
proposed solution

Could your patch please also remove the code that it replaces, or at least deprecate it?

As long as it's not an _ABI_ break, it should be fine.

Created attachment 312144 [details] [review]
proposed solution v2

OK, I've removed SignalProxy* classes.
I don't know, if the change breaks the ABI, but I think, it's not. Some classes contain members of type SignalProxy, but SignalProxy class doesn't contain any member, so I think it's not ABI breaking change.
Can someone confirm it?

I've tested version 2 (from comment 3).
The nm command does not show any trace of SignalProxy# or SignalProxyDetailed#
in the .so files in either glibmm or gtkmm. (# = 0..6)

Test sequence:
1. 'make check' in gtkmm and gtkmm-documentation.
   Passed
2. Apply the patch in glibmm. 'make && make install && make check' in glibmm.
   Passed
3. Without recompiling anything in gtkmm and gtkmm-documentation, run some
   programs in gtkmm and gtkmm-documentation.
   Passed
4. Build gtkmm.
   Failed because Glib::SignalProxy# does not exist.
5. Force rebuild of generated code in gtkmm. (Remove src/.stamps directories.)
   Failed because atkmm uses Glib::SignalProxy#.
6. Force rebuild of generated code in atkmm. (Remove src/.stamps directory.)
   Passed
7. Build gtkmm.
   Failed because Glib::SignalProxy# is used in hand-coded code.
8. Replace SignalProxy# by SignalProxy, 'make && make install' in gtkmm.
   Passed
9. Without 'make check' in gtkmm and gtkmm-documentation, run some programs in
   gtkmm and gtkmm-documentation.
   Passed
10. 'make check' in gtkmm and gtkmm-documentation, then run some programs in
    gtkmm and gtkmm-documentation.
   Passed

Conclusion
----------
It's no ABI break, but of course it's an API break. The API break affects several
mm modules, but probably very few, if any, application programs. I think very
few apps use SignalProxy# directly in their own code.


I also tested with a bunch of alias templates in signalproxy.h. Then no API is
removed. The effect on other packages is insignificant.

template <typename R>
using SignalProxy0 = SignalProxy<R>;
template <typename R, typename T1>
using SignalProxy1 = SignalProxy<R, T1>;
template <typename R, typename T1, typename T2>
using SignalProxy2 = SignalProxy<R, T1, T2>;
template <typename R, typename T1, typename T2, typename T3>
using SignalProxy3 = SignalProxy<R, T1, T2, T3>;
template <typename R, typename T1, typename T2, typename T3, typename T4>
using SignalProxy4 = SignalProxy<R, T1, T2, T3, T4>;
template <typename R, typename T1, typename T2, typename T3, typename T4, typename T5>
using SignalProxy5 = SignalProxy<R, T1, T2, T3, T4, T5>;
template <typename R, typename T1, typename T2, typename T3, typename T4, typename T5, typename T6>
using SignalProxy6 = SignalProxy<R, T1, T2, T3, T4, T5, T6>;

template <typename R>
using SignalProxyDetailed0 = SignalProxyDetailedAnyType<R>;
template <typename R, typename T1>
using SignalProxyDetailed1 = SignalProxyDetailedAnyType<R, T1>;
template <typename R, typename T1, typename T2>
using SignalProxyDetailed2 = SignalProxyDetailedAnyType<R, T1, T2>;
template <typename R, typename T1, typename T2, typename T3>
using SignalProxyDetailed3 = SignalProxyDetailedAnyType<R, T1, T2, T3>;
template <typename R, typename T1, typename T2, typename T3, typename T4>
using SignalProxyDetailed4 = SignalProxyDetailedAnyType<R, T1, T2, T3, T4>;
template <typename R, typename T1, typename T2, typename T3, typename T4, typename T5>
using SignalProxyDetailed5 = SignalProxyDetailedAnyType<R, T1, T2, T3, T4, T5>;
template <typename R, typename T1, typename T2, typename T3, typename T4, typename T5, typename T6>
using SignalProxyDetailed6 = SignalProxyDetailedAnyType<R, T1, T2, T3, T4, T5, T6>;

A few details ought to be changed:

- When signalproxy.h is not generated, it must be included in
  glibmm_files_extra_h in glib/glibmm/filelist.am. Otherwise it won't be
  distributed.

- The first line in signalproxy.h says
  /* This is a generated file, do not edit.  Generated from signalproxy.h.m4 */
  That's not true any more. It should be replaced by the usual copyright and
  license notice.

Created attachment 316451 [details] [review]
proposed solution v3

Thanks Kjell for the review. I've mended a patch. If everything is ok, I'll push it.

First two trivial comments that can be fixed in a separate commit, if you prefer
not to make a 4th version.

- One comment in glib/glibmm/signalproxy.h and one comment in tools/m4/signal.m4
  mention SignalProxy0<void>. Should now be SignalProxy<void>.
- We've now changed all(?) null pointers in glibmm from 0 to nullptr.
  That should be done also in signalproxy.h:
    // Return 0 if the connection is blocked.
    return (!pConnectionNode->slot_.blocked()) ? &pConnectionNode->slot_ : 0;

Then a more important question that I leave to Murray:

Shall the 2*7 alias templates (see comment 4) be included in signalproxy.h?
Then handwritten code with SignalProxy# in other mm modules need not be changed
immediately. If they are included now, they can be deprecated or removed later.

(In reply to Kjell Ahlstedt from comment #6)
> Then a more important question that I leave to Murray:
> 
> Shall the 2*7 alias templates (see comment 4) be included in signalproxy.h?
> Then handwritten code with SignalProxy# in other mm modules need not be
> changed
> immediately. If they are included now, they can be deprecated or removed
> later.

So they'd still be available at runtime for apps that were built against older libsigc++, though newly-built apps wouldn't use them? And there would be no conflict or ambiguity in our API? Sounds good.

This bug concerns Glib::SignalProxy# and Glib::SignalProxyDetailed# (# = 0..6).
As far as I can see, they are not part of the ABI, neither in glibmm nor in
gtkmm, probably because they are templates. (It does not go without saying that
names of templates are not part of any ABI. See e.g. libsigc++ bug 753612.)

If SignalProxy#<> and SignalProxyDetailed#<> are replaced by the variadic
templates SignalProxy<> and SignalProxyDetailedAnyType<>, API would be broken.
With the alias templates near the end of comment 4, this API break is avoided.
The old class names would still be available at compile-time.

Btw the name SignalProxyDetailedAnyType is not perfect. Unfortunately there is
a non-template base class named SignalProxyDetailed. Its name can't be changed
without breaking ABI.

Sounds great. Please go for it.

Why has this bug come to a halt? Are Marcin and me waiting for each other to
continue? My expectation was (and is) that Marcin will push solution v3 or
something very similar, probably with the alias templates in comment 4 added.

That's my fault, I was quite busy last time and completely forgot about this bug. I'll look at this at the evening.

Pushed.

Looks great. Closing this bug.