Re: [Fwd: Re: UnixWare 7.1.3 (BETA), C99 compiler,

Dave,
Thanks for the details, I've copied this reply back to the PostgreSQL
guys as well.

LER

On Mon, 2002-10-28 at 09:00, Dave Prosser wrote:
> Larry Rosenman wrote:
> > From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
> > To: Larry Rosenman <ler(at)lerctr(dot)org>
> > Cc: pgsql-hackers(at)postgresql(dot)org
> > Subject: Re: [HACKERS] UnixWare 7.1.3 (BETA), C99 compiler, current CVS, error...
> > Date: 26 Oct 2002 11:07:13 -0400
> >
> > Larry Rosenman <ler(at)lerctr(dot)org> writes:
> > > Without specifying the -Xb switch to kill the C99 interpretation of
> > > inline, I get the following from current CVS:
> >
> > > UX:acomp: ERROR: "tuplesort.c", line 1854: "inline" functions cannot use
> > > "static" identifier: myFunctionCall2
> >
> > I don't understand what it's unhappy about. My C99 draft sez
> >
> > [#6] Any function with internal linkage can be an inline
> > function.
> >
> > so the text of the message is surely not what they are really
> > complaining about? Or is the compiler broken?
>
> There is a contraint (i.e., a diagnostic is required) in 6.7.4 Function Specifiers
> that says:
>
> An inline definition of a function with external linkage shall not contain a
> definition of a modifiable object with static storage duration, and shall not
> contain a reference to an identifier with internal linkage.
>
> Line 1854 is
> if (DatumGetBool(myFunctionCall2(sortFunction, datum1, datum2)))
> where myFunctionCall2() is a static function defined above ApplySortFunction().
> It's not the inlinedness--a word?--of myFunctionCall2() that's the problem,
> it's that myFunctionCall2() is static and that ApplySortFunction() is inline'd.
>
> You wrote in your follow up:
> >After reading a little further, it seems that the brain damage is in the
> >standard, not the compiler :-(. It looks like C99's notion of a
> >function that is both global and inline is that you must provide *two*
> >definitions of the function, one marked inline and one not; moreover,
> >these must appear in separate translation units. What in the world were
> >those people smoking? That's a recipe for maintenance problems (edit
> >one definition, forget to edit the other), not to mention completely at
> >variance with the de facto standard behavior of inline that's been
> >around for a long time.
>
> The C committee's view of inline does not match the historic GCC one.
> They were trying to find a middle ground that was fully compatible with
> the C++ inline, while not requiring any fancy code generation tricks.
> In other words, that C could still be compiled with a one-pass compiler.
>
> The motivation for this restriction is to make sure that all instances
> of an inline function that's visible outside of the compilation unit
> are identical. Having the same sequence of tokens isn't good enough
> if there are references to identifiers that could well be different in
> differing compilation units.
>
> Until the open source base (and GCC) get around to matching the C99
> inline model, I generally attempt to compile open source with "cc -Xb"
> as that eliminates recognition of inline as a keyword, and thus doesn't
> get into the issues with the clashes between the two models.
>
> >My inclination is to change the code for ApplySortFunction to look like
> >
> > #if defined(__GNUC__)
> > __inline__
> > #endif
> > int32
> > ApplySortFunction
> >
> >so that the inline optimization only gets done for gcc, which we know
> >interprets inline sanely. Anyone see a better answer?
>
> You've given us one source file. Is ApplySortFunction() really called
> from other files? Another approach, assuming both this and that the
> inlining is viewed as important for its three calls within this file,
> to have a front end version of an internal function. To wit:
>
> static inline int32
> StaticApplySortFunction(FmgrInfo *sortFunction, SortFunctionKind kind,
> Datum datum1, bool isNull1,
> Datum datum2, bool isNull2)
> {
> //etc.
> }
>
> int32
> ApplySortFunction(FmgrInfo *sortFunction, SortFunctionKind kind,
> Datum datum1, bool isNull1,
> Datum datum2, bool isNull2)
> {
> return StaticApplySortFunction(sortFunction, kind, datum1, isNull1, datum2, isNull2);
> }
>
> and change all the existing calls within tuplesort.c to use
> StaticApplySortFunction(). This approach requires no #ifdef's and
> should have the same effect as the existing source with GCC's view
> of inline.
>
> --
> Dave Prosser dfp(at)caldera(dot)com (908)790-2358 The SCO Group, Murray Hill, NJ
--
Larry Rosenman http://www.lerctr.org/~ler
Phone: +1 972-414-9812 E-Mail: ler(at)lerctr(dot)org
US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749

Larry Rosenman <ler(at)lerctr(dot)org> forwards:
>>> My inclination is to change the code for ApplySortFunction to look like
>>>
>>> #if defined(__GNUC__)
>>> __inline__
>>> #endif
>>> int32
>>> ApplySortFunction
>>>
>>> so that the inline optimization only gets done for gcc, which we know
>>> interprets inline sanely. Anyone see a better answer?
>>
>> You've given us one source file. Is ApplySortFunction() really called
>> from other files? Another approach, assuming both this and that the
>> inlining is viewed as important for its three calls within this file,

Yup, both of those are correct: the performance-critical calls are in
the same file, but there are others.

>> to have a front end version of an internal function. To wit:
>>
>> static inline int32
>> StaticApplySortFunction(FmgrInfo *sortFunction, SortFunctionKind kind,
>> Datum datum1, bool isNull1,
>> Datum datum2, bool isNull2)
>> {
>> //etc.
>> }
>>
>> int32
>> ApplySortFunction(FmgrInfo *sortFunction, SortFunctionKind kind,
>> Datum datum1, bool isNull1,
>> Datum datum2, bool isNull2)

Grumble. I suppose we have to do it that way, but it's really pretty
stupid. Also, won't this fall foul of the original restriction
(ApplySortFunction referencing the static function myFunctionCall2)?
If not, why not?

> Until the open source base (and GCC) get around to matching the C99
> inline model,

Don't hold your breath... it looks like a net loss in functionality
for no gain, from where I sit.

regards, tom lane

Tom Lane wrote:
> >> static inline int32
> >> StaticApplySortFunction(FmgrInfo *sortFunction, SortFunctionKind kind,
> >> Datum datum1, bool isNull1,
> >> Datum datum2, bool isNull2)
> >> {
> >> //etc.
> >> }
> >>
> >> int32
> >> ApplySortFunction(FmgrInfo *sortFunction, SortFunctionKind kind,
> >> Datum datum1, bool isNull1,
> >> Datum datum2, bool isNull2)
>
> Grumble. I suppose we have to do it that way, but it's really pretty
> stupid. Also, won't this fall foul of the original restriction
> (ApplySortFunction referencing the static function myFunctionCall2)?
> If not, why not?

The change is that the inline function referencing the identifiers with
internal linkage also has internal linkage -- i.e., they're both static.
Since there can be only one definition of StaticApplySortFunction(),
there's no reason to restrict it's contents.

> > Until the open source base (and GCC) get around to matching the C99
> > inline model,
>
> Don't hold your breath... it looks like a net loss in functionality
> for no gain, from where I sit.

The GCC owners are now much more standards-aware and -driven than they
were 10+ years ago. I believe that they are going to (if not already)
have a way of interpreting "inline" according to the C99 standard. It
will probably not be the default, given the incompatibilities, but it'll
be there, somehow, sometime.

As far as I'm concerned, the C99 inline doesn't do "me" much good either,
as I cannot use it in headers to take the place of function-like macros
in standard headers, either. But, this is what you get with Committees--
specifications that make everyone unhappy at roughly equivalent levels. :-)

Since "inline" (as specified in C99) is pretty much just a hint to the
compiler in much the same way that "register" was in the good old days,
it really shouldn't matter too much one way or another, but feel free
to use it as you desire.

--
Dave Prosser dfp(at)caldera(dot)com (908)790-2358 The SCO Group, Murray Hill, NJ