tr1::shared_ptr not implemented in libc++

Bug #1595488 reported by Patrick Welche on 2016-06-23
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
eCAP
Undecided
Unassigned

Bug Description

libecap/common/memory.h defines libecap::shared_ptr as std::tr1::shared_ptr, and as it was ahead of its time, has the comment:

// TODO: add support for boost pointers if std::tr1 is not available

Time has passed, C++11 released, and shared_ptr made it into the standard. Now, we have hit the opposite scenario for users of libc++: it implements the final C++11 standard, and not the "technical report 1" stopgap.

First attempt was to "use std::shared_ptr if available". This didn't work as g++ will claim it isn't available by default unless called with -std=c++11 (or higher). This lead to breakage, as libecap would build without -std=c++11 and contain the symbol

0000000000005278 T libecap::RegisterHost(std::tr1::shared_ptr<libecap::host::Host> const&)

In my opinion, the main consumer of libecap is squid. Squid requires C++11, so it would then look for RegisterHost(std::shared_ptr...) and fail to link.

It seems therefore that the only way to make everyone happy all of the time is for libecap, like squid, to also require C++11, and turn libecap::shared_ptr into std::shared_ptr.

Patch to follow... (You might like to do the cosmetic rename configure.in -> configure.ac thereafter)

Patrick Welche (prlw1) wrote :

Library probably needs a major number bump which I forgot! (RegisterHost changes)

Alex Rousskov (rousskov) on 2016-06-23
Changed in ecap:
status: New → Confirmed
Alex Rousskov (rousskov) wrote :

> Squid requires C++11

Currently stable Squid (v3) does not require C++11 and many Squid installations cannot yet afford an upgrade to an environment with decent C++11 support. This observation leads us to:

> Library probably needs a major number bump which I forgot!

Agreed. And if we are forced to do a major version bump, we should probably introduce libecap API changes that could not be done without one.

Patrick Welche (prlw1) wrote :

I see in the squid 3.5.19 configure.ac file:

if ! test "x$squid_host_os" = "xmingw" -o "x$squid_host_os" = "xcygwin" -o "x$squid_cv_compiler" = "xclang"; then
  AX_CXX_COMPILE_STDCXX_11([noext],[optional])
fi

which I suppose makes this all even worse: I saw that my squid was being compiled with -std=c++11, and libecap wasn't. If the above test wasn't there, both ecap and squid would have been compiled without -std=c++ and std::tr1::shared_ptr would have been used throughout and we wouldn't have a problem. (Well, libc++ users as opposed to libstdc++ users would as they don't have std::tr1 at all, so removing tr1 is the right thing to do.)

In my patch I used squid-head's AX_CXX_COMPILE_STDCXX_11([noext],[mandatory]) but maybe best would be to change that to [optional] so there is a fighting chance that both libecap and squid see the same version of

000000000000d34a T
+libecap::RegisterHost(std::tr1::shared_ptr<libecap::host::Host> const&)

It's because that library symbol changes that we need the number bump.

The status quo doesn't really work...

Alex Rousskov (rousskov) wrote :

The problem is complex, but I suspect you are making it look messier by mixing two different problems together:

1. Modern host applications are gradually losing the ability to use eCAP because their build environment lacks tr1.

2. The same host application may support environments with tr1 and without.

AFAICT, the correct solution to problem #1 is to migrate libecap from tr1 to C++11. This solution requires a library version bump because it changes API and ABI. We may want to piggyback other eCAP API changes on top of this essentially required API change. Host applications and adapters that do not support C++11 will continue to use the to-be-deprecated libecap v1.0.

After #1 is solved, the correct solution to problem #2 is to adjust such host application to use libecap version that matches their current build environment. For example, if Squid v3.5 is being built with C++11, it should use libecap v1.1; otherwise it should continue to use libecap v1.0.

What do you think? Will the above solutions address all the relevant problems that we know about?

Patrick Welche (prlw1) wrote :

That all sounds great! One additional comment:

The bool(theServer) change works for a test program which includes <tr1/memory> and is compiled with -std=c++98, so I think it is safe and will fix up() in a c++11 future. I think that was the only suggested squid change.

(From the build www.pkgsrc.org on clang/llvm/stdc++ point of view, I patched it up to always use C++11 - new package, so no backwards compatibility issues - so the originator of this bug report should be able to use squid+ecap by building from pkgsrc in the meantime.)

Patrick Welche (prlw1) wrote :

I got my bugs mixed up - ignore the last paragraph

Alex Rousskov (rousskov) wrote :

For the record, a related Squid bug is here: http://bugs.squid-cache.org/show_bug.cgi?id=4376

Patrick Welche (prlw1) wrote :

Just wondering what libecap v1.1 might look like in the brave new c++11 world?
("piggyback other eCAP API changes on top of this")

Alex Rousskov (rousskov) wrote :

I do not have a list of changes that is polished enough to share. Suggestions welcomed (but not inside this bug report!).

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.