Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Pedro Alves <palves@redhat.com>
Cc: tom@tromey.com, simon.marchi@polymtl.ca,	gdb-patches@sourceware.org
Subject: Re: [RFA 09/22] Remove make_cleanup_restore_current_ui
Date: Thu, 13 Oct 2016 14:46:00 -0000	[thread overview]
Message-ID: <83lgxs8fyj.fsf@gnu.org> (raw)
In-Reply-To: <a9ccb080-8727-72fe-f579-d21c204ed4c8@redhat.com> (message from	Pedro Alves on Thu, 13 Oct 2016 15:26:07 +0100)

> Cc: tom@tromey.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 13 Oct 2016 15:26:07 +0100
> 
> On 10/13/2016 02:52 PM, Eli Zaretskii wrote:
> 
> > Since I'm still unclear what is the oldest C++ standard we want to
> > support, I cannot decide whether I agree or disagree.  Is the version
> > of the C++ standard we support C++03 or C++11?  My comment assumed
> > that it was C++11.
> 
> It was decided in 2014 that we'd target C++03.  We have not decided
> yet to _require_ C++11.

Then that comment is not yet relevant.  But then I think we should not
use C++11 code in GDB.

> >> I'll give you more examples of why I think forbidding C++11 completely
> >> is short sighted.
> > 
> > Thanks, but that kind of arguments completely misses the point.  You
> > don't need to convince me that a later standard is better than the
> > previous ones.  The issue at hand is whether we should write code that
> > targets more than a single language standard, where these standards
> > aren't compatible.  I think we shouldn't.
> 
> They are not incompatible.  C++03 code compiles with a C++11 compiler
> just fine.

Granted, I meant the other way around: use code that a C++11 compiler
will compile, but not a C++03 compiler.

> > The code obfuscation is very significant.  Of course, as long as you
> > don't look into the gnulib headers, you don't care, but in this case
> > you propose to have all those shims in our sources, in plain sight.
> 
> Yes, "all those shims".  One file.  And then cleaner code in thousands
> of uses of the shim throughout the codebase.  None of that
> shim-using code needs to know what version of the standard is
> being targeted.  Take a look at patch #3 from the gdb::unique_ptr
> series, and see for yourself.
> 
> So I'll take that tradeoff, yes.

Is this a one-time tradeoff, for this single patch?  Or is this a
policy?  In the former case, I have no objections.  In the latter
case, the "one file" argument doesn't apply.

> >>> that means maintenance burden 
> >>
> >> For whom?  _I'm_ willing to maintain this code.
> > 
> > Are you saying that no one else will be allowed to modify the code
> > which has 2 branches, one for C++03, the other for C++11?  I'm betting
> > you aren't saying that, 
> 
> Of course not.
> 
> > and so this is a burden for all of us, not just for you.  
> 
> You're automatically assuming it's a burden.  I believe that's
> false.

It should be clear and agreed by all that maintaining two
implementations for a single feature is more work than maintaining
just one.

> >> It's not rocket science code.
> > 
> > Today it isn't.  I'm not talking about this particular patch, I'm
> > talking about this policy in general.  As do you, I suppose: you
> > aren't just asking for agreement about using C++11 features in this
> > single case, right?
> 
> I'm talking about adding a small utility that is going to be used 
> extensively throughout the codebase.  A smart pointer is a central widget.
> That mine was modeled on a C++11 feature should be seen as
> a _good_ thing.  The std::unique_ptr was so good that it made it
> to the standard.  Not a small feat.  Bonus.  Once we move on to C++11
> for real (I don't know when), then there's one less API to learn.

And I'm again asking whether this is about this single patch, or about
a more general policy.  I assume that it's the latter, in which case
we are not talking about a single small utility, we are talking about
all the code that will be in the future admitted to GDB with the same
premise.  It is the policy that I object to, not a single exception.

> >> Several others have said they think this is a good idea.  I've even
> >> ran this idea through the libstdc++ maintainer in person at the Cauldron.
> >> Trevor said he wants to do this in gcc as well.
> > 
> > I just wanted to voice my opposition, that's all.  I don't have to
> > give up just because a few others think otherwise.  Right?
> 
> Of course.  The problem is that your opinion is interpreted as
> a hard blocker.  The result is stalling.

If there are no more convincing arguments, then a usual way out of
stalling is to find some compromise.  Is that possible in this case?

> >> If redirecting to C++11 std::unique_ptr turns out to cause trouble, or
> >> I'm hit by a bus, then it's trivial to remove that redirection.  All it
> >> takes is remove a handful of lines of code guarded with
> >> #if __cplusplus >= 201103 to always go through the fallback implementation.
> > 
> > Once again, it's not just this single patch that bothers me.  Once we
> > have enough of these #if's, removing them is not necessarily a trivial
> 
> And not "not necessarily" either...  This just looks like fear of
> the unknown, I'm afraid.

What exactly is unknown here?  It should be clear to anyone that more
of such patches, with separate implementations for C++11 and C++03,
will come soon enough.  What reason do you have to think they won't?
After all, C++11 is so much better than C++03, right?

> > matter, especially when most of the builds, perhaps even all of them,
> > have been using the C++11 code path all the time, and the other one
> > has simply bitrotted.
> 
> As I've said before, we can make use of the buildbot for that.
> If the fallback code breaks, you get an immediate email notification.

If someone sets it up to build both with and without C++11, yes.
A.k.a. "maintenance burden".


  reply	other threads:[~2016-10-13 14:46 UTC|newest]

Thread overview: 121+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-27  4:49 [RFA 00/22] More C++-ification Tom Tromey
2016-09-27  4:41 ` [RFA 07/22] Change scoped_minimal_symbol_reader to store objfile Tom Tromey
2016-09-29  9:19   ` Trevor Saunders
2016-09-30 21:41     ` Tom Tromey
2016-09-27  4:41 ` [RFA 15/22] Use std::string in macho_symfile_read_all_oso Tom Tromey
2016-10-09 17:28   ` Pedro Alves
2016-10-10 22:40     ` Tom Tromey
2016-10-10 22:46       ` Pedro Alves
2016-09-27  4:42 ` [RFA 19/22] Convert tid_range_parser to class Tom Tromey
2016-09-30  1:41   ` Pedro Alves
2016-09-30 14:52     ` Tom Tromey
     [not found]       ` <926126cb-b3c5-340b-ac1c-5bc14ca41bf9@redhat.com>
2016-10-04 19:24         ` Pedro Alves
2016-10-04 23:09           ` Pedro Alves
2016-10-05  2:16             ` Trevor Saunders
2016-10-12  2:12             ` Tom Tromey
2016-10-13  1:06               ` Pedro Alves
2016-09-27  4:42 ` [RFA 16/22] Use std::vector in elf_read_minimal_symbols Tom Tromey
2016-10-09 17:30   ` Pedro Alves
2016-09-27  4:43 ` [RFA 20/22] Initial conversion of dwarf_expr_ctx Tom Tromey
2016-10-09 17:40   ` Pedro Alves
2016-09-27  4:45 ` [RFA 21/22] Convert DWARF expr functions to methods Tom Tromey
2016-10-09 19:18   ` Pedro Alves
2016-09-27  4:47 ` [RFA 05/22] Turn wchar iterator into a class Tom Tromey
2016-10-06  1:01   ` Pedro Alves
2016-09-27  4:47 ` [RFA 22/22] Convert dwarf_expr_context_funcs to methods Tom Tromey
2016-10-09 19:11   ` Pedro Alves
2016-10-10 18:31     ` Pedro Alves
2016-10-10 19:33       ` Pedro Alves
2016-09-27  4:47 ` [RFA 04/22] Use scoped_restore for current_ui Tom Tromey
2016-09-27  4:47 ` [RFA 02/22] Use RAII to save and restore scalars Tom Tromey
2016-09-27 10:24   ` Trevor Saunders
2016-09-30  1:40     ` Pedro Alves
2016-09-30  9:22       ` Pedro Alves
2016-09-30 15:00       ` Tom Tromey
2016-09-30 23:50         ` Pedro Alves
2016-09-30 15:44       ` Tom Tromey
2016-09-30 23:51         ` Pedro Alves
2016-10-01  3:55           ` Tom Tromey
2016-10-01  4:23             ` Tom Tromey
2016-10-01 10:33               ` Pedro Alves
2016-10-02 17:11                 ` Tom Tromey
2016-10-05  0:06                   ` Pedro Alves
2016-10-12 22:36                     ` Tom Tromey
2016-09-27  4:48 ` [RFA 09/22] Remove make_cleanup_restore_current_ui Tom Tromey
2016-09-29 11:55   ` Trevor Saunders
2016-10-01  3:47     ` Tom Tromey
2016-10-01  4:29   ` Simon Marchi
2016-10-06  2:53     ` Tom Tromey
2016-10-06  1:24   ` Pedro Alves
2016-10-06  2:52     ` Tom Tromey
2016-10-09  4:31   ` Simon Marchi
2016-10-09 15:10     ` Tom Tromey
2016-10-09 19:20       ` Pedro Alves
2016-10-12 22:43         ` Tom Tromey
2016-10-13  1:28           ` Pedro Alves
2016-10-13  6:11             ` Eli Zaretskii
2016-10-13 10:16               ` Pedro Alves
2016-10-13 13:53                 ` Eli Zaretskii
2016-10-13 14:26                   ` Pedro Alves
2016-10-13 14:46                     ` Eli Zaretskii [this message]
     [not found]                       ` <9d9dca17-56a6-6c0a-44bb-efc425f24d8d@redhat.com>
2016-10-13 15:19                         ` Eli Zaretskii
2016-10-13 15:43                           ` Pedro Alves
2016-10-13 15:48                             ` Pedro Alves
2016-10-17 23:43                             ` Go C++11? (was: Re: [RFA 09/22] Remove make_cleanup_restore_current_ui) Pedro Alves
2016-10-18  6:14                               ` Eli Zaretskii
2016-10-19 18:02                               ` Go C++11? Luis Machado
2016-10-19 22:34                                 ` Pedro Alves
2016-10-13 15:27                       ` [RFA 09/22] Remove make_cleanup_restore_current_ui Pedro Alves
2016-10-13 14:26                 ` Trevor Saunders
2016-09-27  4:48 ` [RFA 01/22] Change selttest.c to use use std::vector Tom Tromey
2016-09-27  8:50   ` Trevor Saunders
2016-09-27 16:44     ` Tom Tromey
2016-09-28 14:58       ` Trevor Saunders
2016-09-29  8:59         ` Tom Tromey
2016-10-06  0:18       ` Pedro Alves
2016-10-06  0:39       ` Pedro Alves
2016-09-30 14:43   ` Simon Marchi
2016-09-30 21:40     ` Tom Tromey
2016-10-06  0:45       ` Pedro Alves
2016-09-27  4:48 ` [RFA 03/22] Use scoped_restore for ui_file Tom Tromey
2016-10-01  4:28   ` Simon Marchi
2016-10-01  5:22     ` Tom Tromey
2016-10-01 11:47       ` Simon Marchi
2016-10-13 14:56         ` Tom Tromey
2016-09-27  4:48 ` [RFA 08/22] Record minimal symbols directly in reader Tom Tromey
2016-10-01  4:29   ` Simon Marchi
2016-10-06  1:12     ` Pedro Alves
2016-09-27  4:50 ` [RFA 17/22] Remove make_cleanup_restore_current_uiout Tom Tromey
2016-09-29 14:35   ` Trevor Saunders
2016-09-29 15:23     ` Tom Tromey
2016-09-29 17:55       ` Simon Marchi
2016-09-29 20:34         ` Tom Tromey
2016-09-30  2:45       ` Pedro Alves
2016-09-30 23:48         ` Tom Tromey
2016-09-30 23:52           ` Pedro Alves
2016-09-27  4:51 ` [RFA 12/22] Remove unnecessary null_cleanup Tom Tromey
2016-10-09 17:06   ` Pedro Alves
2016-09-27  4:51 ` [RFA 13/22] Remove unnecessary cleanup from stabsread.c Tom Tromey
2016-09-30 16:19   ` Tom Tromey
2016-10-09 17:07   ` Pedro Alves
2016-09-27  4:51 ` [RFA 18/22] Some cleanup removal in dwarf2loc.c Tom Tromey
2016-10-09 17:37   ` Pedro Alves
2016-09-27  4:51 ` [RFA 14/22] Replace two xmallocs with vector Tom Tromey
2016-10-09 17:20   ` Pedro Alves
2016-10-12 22:39     ` Tom Tromey
2016-10-13  1:17       ` Pedro Alves
2016-10-13  2:04         ` Tom Tromey
2016-10-13 15:15     ` Tom Tromey
2016-10-13 15:26       ` Trevor Saunders
2016-09-27  4:52 ` [RFA 10/22] Remove some cleanups in MI Tom Tromey
2016-10-06  1:42   ` Pedro Alves
2016-09-27  4:52 ` [RFA 06/22] Introduce scoped_minimal_symbol_reader Tom Tromey
2016-10-06  1:10   ` Pedro Alves
2016-10-06 15:37     ` Tom Tromey
2016-10-10 23:06     ` Tom Tromey
2016-10-10 23:26       ` Pedro Alves
2016-09-27  8:32 ` [RFA 11/22] Change command stats reporting to use class Tom Tromey
2016-10-09 17:01   ` Pedro Alves
2016-10-11 17:31     ` Tom Tromey
     [not found] ` <e06fbe1ea266e078eaff6bdc98e48efa@simark.ca>
2016-10-08 16:42   ` [RFA 00/22] More C++-ification Pedro Alves
2016-10-09  2:09   ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83lgxs8fyj.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    --cc=simon.marchi@polymtl.ca \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox