Re: [DynInst_API:] RFC: require users to have C++11


Date: Wed, 31 Aug 2016 14:07:21 -0500
From: "Mark W. Krentel" <krentel@xxxxxxxxxxx>
Subject: Re: [DynInst_API:] RFC: require users to have C++11
Ok, I've been sufficiently taunted.  Speaking as someone who builds a
subset of Dyninst (parseapi + symtabapi) from source and applications
that use it (Rice HPCToolkit) on a daily basis ....

I'm a bit unsure of just what you're suggesting.

First, I think it's perfectly ok to require Dyninst clients to build
their apps with essentially the same CXXFLAGS (-std=c++11 or
-std=c++0x) that you use to build Dyninst.  I'm surprised that you
even try to do otherwise.  In HPCToolkit, we already do this and have
for a long time.

And be careful you don't fall into a trap where you distribute
pre-built libraries with much newer features and provide compatibility
with header file adjustments.  If I can't build Dyninst from source on
the same system that I'm building my application, then that does me no
good.

Second, the real question is where do you draw the line.  As you know,
there are still a lot systems with RHEL/CentOS 6.x with gcc 4.4 and
standard c++0x, not full c++11.  And I'd prefer not to lose them just
yet.

In general, my response to questions like this is, "You do what you
think is best and we'll adapt."  I know that gcc 4.4 is very old and
some day it won't be feasible to support that anymore.  But I'd prefer
that we're not there just yet.  Are we there now?

Btw, it's not always reliable to test the compiler version to
determine what features it supports, not even for gcc.  It's more
robust to try compiling a code snippet at configure time.

For example, a long time ago in Dyninst 6.1, you tested for
<tr1/unordered_set> in dyntypes.h by testing for gcc >= 4.3.

   #if (__GNUC__ > 4) || \
      (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)
      //**************** GCC >= 4.3.0 ***********
      #include <tr1/unordered_set>
      #include <tr1/unordered_map>

That mostly worked, except on our powerpc cluster with gcc 4.3.0 that
did not support unordered_map or unordered_set.

Even today, I can cite examples where Boost gets the wrong answer for
the gcc 4.7.0 cross-compiler for KNC.  I have to add a patch for
-DBOOST_NO_CXX11_ALLOCATOR and -DBOOST_NO_CXX11_VARIADIC_TEMPLATES on
KNC.

It's a harder problem for Boost because they're mostly headers and
usually don't have a config step.

--Mark



On 08/31/16 11:08, Bill Williams wrote:
I won't say it's entirely a fool's errand to expect to build Dyninst mutators with a less capable compiler than one uses to build Dyninst itself, but it seems not to be a terribly valuable form of backwards compatibility, especially given that our only binary distribution at present is managed by RedHat. :) If I'm wrong on that point, though, folks should speak up.

In general I think I'd prefer to deprecate boost:: in favor of std:: where boost-isms have made it into C++11. I don't have firm notions on a migration timetable, though, and user feedback is more relevant than my preferences about code hygiene here.

--bw

________________________________________
From: Dyninst-api <dyninst-api-bounces@xxxxxxxxxxx> on behalf of Josh Stone <jistone@xxxxxxxxxx>
Sent: Tuesday, August 30, 2016 4:32 PM
To: dyninst-api@xxxxxxxxxxx
Subject: Re: [DynInst_API:] RFC: require users to have C++11

Anyone? Anyone? Bueller?

If we're going to keep pre-C++11 compatibility, we must keep the public
headers clean.  For instance, in commit 895d3fad7e59 I fixed a template
'>>' which was not allowed before C++11.  Now I just noticed another
case of this problem was added in commit dff30ae88e36.

On 08/17/2016 11:47 AM, Josh Stone wrote:
Dyninst already requires C++11 features to build itself, but maintains
support for pre-C++11 in the public interface.  This means for instance
that we have to use std::tr1::unordered_map and boost::shared_ptr
instead of the C++11 std::unordered_map and std::shared_ptr.

For a small example motivation, I was looking at the enormous function
SyscallInformation::SyscallInformation(), which has to fill the mapping
tables.  Right now it does this with individual insert() calls, which
generates a lot of code.  I'm thinking of a few ways to make this more
data-driven, and the easiest would be with C++11 initializer lists:

   Linux_Arch_ppc64_syscallNames.insert({
     {0, "restart_syscall"},
     {1, "exit"},
     {2, "fork"},
     /* ... */
   });

Now, this map is purely internal, so we could just go ahead convert it
from the generic dyn_hash_map to a C++11 std::unordered_map and go
crazy.  But I'd like to have the opportunity to use C++11 throughout the
code, without worrying about what's in public headers.

Assuming we want to maintain RHEL6 support (GCC 4.4), that still limits
us to some earlier "C++0x" features, listed at the link below.  I'm not
sure about the state of Dyninst's minimum MSVC on the Windows side.
https://gcc.gnu.org/gcc-4.4/cxx0x_status.html

FWIW, GCC6 leaped up to -std=gnu++14 as the default mode.  This is in
Fedora 24 now and will be in Ubuntu 16.10.


Could we make this change in master now for Dyninst 9.3?  Wait for 10.0?
_______________________________________________


[← Prev in Thread] Current Thread [Next in Thread→]