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

[Josh Stone <jistone@xxxxxxxxxx>]

On 08/31/2016 12:07 PM, Mark W. Krentel wrote:
> 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 ....

Thanks for your input!

> 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.

Right, so this is the point that I'm suggesting we change.  Despite
using C++11 internally, Dyninst is currently keeping compatibility with
non-C++11 users for the public headers. (or at least trying to)

That was probably helpful when Dyninst first transitioned to C++11.
Now, I think we're far enough along to require it of users too.

> 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.

At least for RHEL/Fedora, we build everything with the system compiler.

> 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.

Yes, I think RHEL6 is still an important target.  Here's that table
again for reference: https://gcc.gnu.org/gcc-4.4/cxx0x_status.html

But that's only language features.  For new libraries, we have to look
at libstdc++, but its online docs only go back to 4.6.4.
https://gcc.gnu.org/onlinedocs/
https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html

If you install the libstdc++-docs rpm, it will be in:
/usr/share/doc/libstdc++-docs-4.4.7/html/manual/status.html

Although GCC isn't the whole world, sigh.  I'm not sure what this level
of support corresponds to in ICC, MSVC, etc.

> 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?

Heck, I work so much in Fedora that even RHEL7 starts to feel old. :)
But yes, I would like Dyninst to still support RHEL6 for a while.

> 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.

Preferably, it would be nice to avoid *all* compiler conditionals, at
least in the public headers.  It's easy for that to get mixed up.  I'd
rather require and assume a certain base (gcc-4.4) and just forbid using
any more than that.

> 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.

This particular case is helped by assuming some C++11/C++0x, since
everyone on >=4.4 can just use std::unordered_map/set.

(Hopefully the cluster you mention doesn't need to still be supported.)

> 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.

And FWIW, assuming C++11 will reduce our Boost dependency.


> 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?
>>> _______________________________________________
> 
> 
> _______________________________________________
> Dyninst-api mailing list
> Dyninst-api@xxxxxxxxxxx
> https://lists.cs.wisc.edu/mailman/listinfo/dyninst-api
>