Re: PATCH to libiberty: Move to the intersection of C90 an C++

[Gabriel Dos Reis <gdr at integrable-solutions dot net>]

Ian Lance Taylor <ian@airs.com> writes:

| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| 
| > -extern size_t	htab_size (htab_t);
| > -extern size_t	htab_elements (htab_t);
| >  extern double	htab_collisions	(htab_t);
| >  
| > +   
| > +/* Return the current size of given hash table. */
| > +
| > +static inline size_t
| > +htab_size (htab_t htab)
| > +{
| > +  return htab->size;
| > +}
| > +
| > +/* Return the current number of elements in given hash table. */
| > +
| > +static inline size_t
| > +htab_elements (htab_t htab)
| > +{
| > +  return htab->n_elements - htab->n_deleted;
| > +}
| > +
| 
| This seems like an unrelated change.  Why make it?


I tried to explain it in the previous message:

  htab_size() and htab_elements() are now static inline, instead of
  using the "pseudo-semantics" of extern inline (problem discovered when
  building the GNU C compiler with g++). 

But, that is indeed poor.  So, let me try again.
Currently (i.e. without my patch), they are defined as

   inline size_t
   htab_size (htab_t htab)
   {
     return htab->size;
   }

   inline size_t
   htab_elements (htab_t htab)
   {
     return htab->n_elements - htab->n_deleted;
   }

With a C90 compiler that does not understand inline, these are OK
because we have

    #define  inline /* nothing/

With a C compiler (like gcc), this is supposed to have the semantics of
"extern inline".

With a C++ compiler, these become inline function in hashtab.c,
therefore ought to be declared inline in every translation unit where
they are declared, and defined in every translation unit where they
are used.

I supposed that it was for performance reason why they were
defined inline in hashtab.c, which is why I moved them to hashtab.h
with the "static inline" marker.  They won't pose any problem with a
C90 compiler that does not understand inline, and they won't cause any
problem with a C90 compiler that does.  I'm also willing to turn them
into macros if you prefer.

|  In libiberty we
| try to be backward compatible to C90 compilers, and they won't
| understand inline. 

I understand that, but the C90 compiler will never see "inline" if it
does not understand it because we have the following in ansidecl.h

/* inline requires special treatment; it's in C99, and GCC >=2.7 supports
   it too, but it's not in C89.  */
#undef inline
#if __STDC_VERSION__ > 199901L
/* it's a keyword */
#else
# if GCC_VERSION >= 2007
#  define inline __inline__   /* __inline__ prevents -pedantic warnings */
# else
#  define inline  /* nothing */
# endif
#endif

| If there is a performance advantage here, let's
| just use a preprocessor macro.  Otherwise, I don't think this is
| related to converting to C++.

I'm not converting to C++, just moving to the intersection of C90 and
C++ :-).  The relation is that the function becomes "inline" in C++
and that means it needs definition in every translation unit.

| I see that you removed the functions in hashtab.c.  I think we might
| just as well not mark them inline in hashtab.c, if that is what it
| takes to fix this for C++.

Would you reconsider your comments given the above explanation?

-- Gaby