Subject: RE: gsfr

From: "Moore, Paul" <[email-masked]>
Date: Tue, 17 Nov 1998 13:31:01 -0000

>From: 	[email-masked][SMTP:[email-masked]]
>
>On Mon, Nov 16, 1998 at 01:50:45PM -0500, [email-masked] wrote:
>>   dv> It 'clearly sets the sticky attribute for window win' until there
>>   dv> is a problem that you can easily solve by adding another line to
>>   dv> the macro.  Then you'll have this line 9 times out of ten in
>>   dv> places where it isn't needed.
>> 
>> That is obviously terrible code.  If people are going to write bad code,
>> then they'll write bad code.  I'm not sure we should forgo any possible
>> advantages in readability and maintainability this might give us because
>> of that.
>> 
>> But this, definitely, is the crux of the argument so far.
>
>In this case I don't see better readability. And it should be
>unnecessary to maintain that if the design is done properly.

(I am mostly just lurking here, but I can't let this discussion go by
without comment...)

Abstraction, and separation of interface and implementation, are pretty
much universally accepted as a good thing. They have to be applied with
some taste (just like commenting, where ++i; /* increment i */ is
unhelpful), but I would say that in this case, where we are changing an
implementation and finding it hard because of the *lack* of separation
of the interface, it would be foolish to perpetuate the problem by not
abstracting out the interface while we are making the changes.

>>  Anyway, I'm just pointing out that between these two:
>> 
>>   SetSticky(win);
>>   win->sticky = 1;
>> 
>> it's not obvious to me that the second is superior, in terms of
>> understanding what it does, than the first.
>
>I don't trust names. They may suggest things that are not done.
>This is a frequent cause of confusion with macros.

[I think the attributions are wrong here, I've lost the thread of all
this... But the point remains the same]

This comment applies EXACTLY as well for functions, as for macros. But I
don't think we're arguing here that functions are a bad thing. The
question is simply whether encapsulating style-setting type operations
is a sufficiently useful abstraction to be worth doing. If functions
offered zero overhead compared to the inline code, then we'd be talking
about functions. However, as they don't, we have to use macros as a poor
man's substitute.

Further, the issue here is not about macros in general - it's about ONE
PARTICULAR style of macro usage - the case where macros are used to
emulate inline functions. If the C optimiser had a god-like ability to
decide on the fly whether to inline a function or to use it out of line,
then we could just write a function

    void SetSticky(Window *win)
    {
        win->sticky = 1;
    }

and we'd be OK. If we were using C++, we could make this an inline
function and have most of the benefits (inline functions can cause code
bloat in some cases, where the optimiser isn't good enough). But we're
using C, so we have macros, which have to be written carefully to get
the same benefit. (And which can hide strange problems if badly written
- but if they are big enough to risk problems like this, they should be
(non-inline) functions and not macros). I'm assuming that everyone
changing the code is competent enough to avoid the well-known problems
(by tricks like parenthesising everything) and would do

    #define SetSticky(win) (void)(((win)->sticky) = 1)

and not

    #define SetSticky(win) win->sticky = 1

or

    #define SetSticky(win) { win->sticky = 1; }

This is just an implementation issue of macros, well known to C
programmers, and shouldn't be a difficulty.

[PS - I agree that macros should be all-capitals, although I'm less
convinced in cases like these, where we'd write a normal function except
for efficiency concerns].

Of course, I also assume that the call SetSticky(win), however
implemented, can be trusted to do what it says (set the sticky flag),
and nothing else. Dominik seems to have some difficulty trusting code
this far, which seems to me to be unreasonable - if we can't trust the
documentation (and I consider function names to be documentation) then
we are totally lost.

>> Also, I'm not sure I agree with the argument that implementation- and
>> data-hiding is useless because if the design was right it wouldn't be
>> necessary :).  In changing environments and requirements, the design may
>> need to change.  The ability to do this with minimal disruption to code
>> which doesn't need to know about the change is the hallmark of a good
>> implementation.
>
>I don't say it's useless. I just hate macros to do it.

If we had an alternative to macros, I'd agree that we should use it.
It's a real pity that C hasn't got inline functions (especially as gcc
has got them, but being non-standard we can't use them :-() But we
don't, so we should use macros (and document clearly how and why we're
using them, so the principle remains clean).

Saying that we shouldn't encapsulate because the only implementation
technique we have is prone to problems in the wrong hands, is a case of
throwing the baby out with the bathwater...

>I am willing to give it a try, but I'm sceptical. In this case it
>may be reasonable after all.

It's a matter of being clear WHY and HOW we are using macros, and
keeping the boundaries clear. In my view, any inline-function-substitute
macro which needs tricks like do { ... } while (0) to combine multiple
statements is suspect - we should limit ourselves to SINGLE expressions.

And I strongly advise against macros like

    #define MyMacro()  stmt1; \
                      stmt2; \
                      stmt3;

These simply don't work like functions in all contexts. Use comma
expressions, and ?: for conditions, and remember to cast to void any
macro expression which is intended to act like a void function. And go
no further...

>>   dv> And we don't even have *one* authority on which style of
>>   dv> code/patches makes it into CVS.
>> 
>> We have better than that, we have many people with "authority" :).
>
>That's no advantage in respect to coding style. Many people ==>
>different styles ==> noone bothers about the style of patches (?).

Agreed. But we should therefore document our practices to help share
style. And we shouldn't be afraid of querying a patch on stylistic
grounds ("wouldn't it be better to code that construct like this?").

>>... 
>> There is no check that the compiler will or will not make differently on
>> code using a macro than it would make on the same code with the macro
>> expanded (since the compiler is seeing the code with the macro expanded).
>
>#define SETCHARPOINTER(c, p) { p = (char *)&c; }
>
>{
>  int i;
>  int *pi;
>
>  SETCHARPOINTER(i, pi);
>}
>
>That's what I mean.

That's simply a badly written macro. What's wrong with

    #define SETCHARPOINTER(c,p) (void)((p) = &(c))

Note that using an expression cast to void allows correct use in the if
() SETCHARPOINTER(); else situation, and parenthesising p and c avoids
precedence problems. And the compiler will type check the assignment
correctly.

And anyway, this is NOT a case of a macro emulating an inline function -
you can't write SETCHARPOINTER as a function. I *don't* support (in
general) macros which are syntactically or semantically different from
"pure" C. Unless they are clearly documented, and VERY carefully written
to act as closely to normal statements as possible - see the if...else
case above. If you are worried about this style of macro, I would
generally support you
(but remember we didn't need nastinesses like pass-by-reference for
SetStcky, above).

Back to lurking, until I can put my code where my mouth is...
Paul Moore.
--
Visit the official FVWM web page at <URL:http://www.hpc.uh.edu/fvwm/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked].