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

[Daniel Berlin <dberlin@redhat.com>]

Daniel Berlin <dberlin@redhat.com> writes:

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

I actually copied/pasted the wrong macro, i meant:

#define SYMBOL_INIT_DEMANGLED_NAME(symbol,obstack)			\
  do {									\
    char *demangled = NULL;						\
    if (SYMBOL_LANGUAGE (symbol) == language_unknown)			\
	    SYMBOL_LANGUAGE(symbol) = language_auto;			\
    if (SYMBOL_LANGUAGE (symbol) == language_cplus			\
	|| SYMBOL_LANGUAGE (symbol) == language_auto)			\
      {									\
	demangled =							\
	  cplus_demangle (SYMBOL_NAME (symbol), DMGL_PARAMS | DMGL_ANSI);\
	if (demangled != NULL)						\
	  {								\
	    SYMBOL_LANGUAGE (symbol) = language_cplus;			\
	    SYMBOL_CPLUS_DEMANGLED_NAME (symbol) = 			\
	      obsavestring (demangled, strlen (demangled), (obstack));	\
	    free (demangled);						\
	  }								\
	else								\
	  {								\
	    SYMBOL_CPLUS_DEMANGLED_NAME (symbol) = NULL;		\
	  }								\
      }									\
    if (SYMBOL_LANGUAGE (symbol) == language_java)			\
      {									\
	demangled =							\
	  cplus_demangle (SYMBOL_NAME (symbol),				\
			  DMGL_PARAMS | DMGL_ANSI | DMGL_JAVA);		\
	if (demangled != NULL)						\
	  {								\
	    SYMBOL_LANGUAGE (symbol) = language_java;			\
	    SYMBOL_CPLUS_DEMANGLED_NAME (symbol) = 			\
	      obsavestring (demangled, strlen (demangled), (obstack));	\
	    free (demangled);						\
	  }								\
	else								\
	  {								\
	    SYMBOL_CPLUS_DEMANGLED_NAME (symbol) = NULL;		\
	  }								\
      }									\
    if (demangled == NULL						\
	&& (SYMBOL_LANGUAGE (symbol) == language_chill			\
	    || SYMBOL_LANGUAGE (symbol) == language_auto))		\
      {									\
	demangled =							\
	  chill_demangle (SYMBOL_NAME (symbol));			\
	if (demangled != NULL)						\
	  {								\
	    SYMBOL_LANGUAGE (symbol) = language_chill;			\
	    SYMBOL_CHILL_DEMANGLED_NAME (symbol) = 			\
	      obsavestring (demangled, strlen (demangled), (obstack));	\
	    free (demangled);						\
	  }								\
	else								\
	  {								\
	    SYMBOL_CHILL_DEMANGLED_NAME (symbol) = NULL;		\
	  }								\
      }									\
  } while(0)


As you can see, the do/while is clearly there.


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