This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [PATCH]: C++ mangling patch that is about to be committed


Kevin Buettner <kevinb@cygnus.com> writes:

> On Oct 10, 11:55am, Daniel Berlin wrote:
> 
> > Elena Zannoni <ezannoni@cygnus.com> writes:
> >
> > > BTW, I thought
> > > we agreed to leave the do--while construct in the
> > > SYMBOL_INIT_DEMANGLED_NAME macro.
> > I'd rather not.
> > It's not used in if statements, and *never* should be.
> > The argument that someone, someday, might want to, just isn't
> > convincing, because they shouldn't.
> 
> Daniel,
> 
> This is really not the way to handle this kind of change.  Elena is
> right.  The consensus was to leave the ``do ...  while (0)'' construct
> in the SYMBOL_INIT_DEMANGLED_NAME macro. 

No, there was no consensus, we only really discussed *why* it existed that
way. Nobody provided a convincing reason to keep it the old way, except "It
was that way". 

> I believe that turning it
> into a proper function was also discussed and is regarded as a viable,
> perhaps even superior, alternative.

Feel free to do this.
I'm not playing with symbol tables anymore, I don't have time to wait
for my patches to get approved.

> 
> I know that you don't like the do ... while (0) construct, but it is
> not right for you to try to sneak changes eliminating it past the
> maintainer multiple times, particularly when you agreed to leave it
> in.

I haven't tried to sneak it in, the elimination exists in every
repository i've applied any version of the C++ patches to, so i have to hand
patch every single diff I generate to put it back.

It's not like I can just magically make it reappear in all the
repository copies I have around.

The reality is it exists in the one repository that matters in this
case:
#define SYMBOL_INIT_LANGUAGE_SPECIFIC(symbol,language)                  \
  do {                                                                  \
    SYMBOL_LANGUAGE (symbol) = language;                                \
    if (SYMBOL_LANGUAGE (symbol) == language_cplus                      \
        || SYMBOL_LANGUAGE (symbol) == language_java                    \
        )                                                               \
      {                                                                 \
        SYMBOL_CPLUS_DEMANGLED_NAME (symbol) = NULL;                    \
      }                                                                 \
    else if (SYMBOL_LANGUAGE (symbol) == language_chill)                \
      {                                                                 \
        SYMBOL_CHILL_DEMANGLED_NAME (symbol) = NULL;                    \
      }                                                                 \
    else                                                                \
      {                                                                 \
        memset (&(symbol)->ginfo.language_specific, 0,                  \
                sizeof ((symbol)->ginfo.language_specific));            \
      }                                                                 \
  } while (0)
                    


I really resent the implication.
I split the patches and did modifications based on the existing set of
patches, which is why the patch has it.

> 
> Kevin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]