Re: addendum to gcc/config/i386/i386.c patch

[Jeffrey A Law <law at cygnus dot com>]

  In message <199810251643.RAA03494@jet.franken.de>you write:
  > Hi.
  > 
  > As Addendum to my patch the 'misbehaviour' it tries to fix.
  > [I though it was obvious, but it (probably) isn't. Sorry.]
There are several problems with your patch that you need to fix.  First, the
formatting is incorrect, particularly in your use of whitespace and over-parens.
For example, you had one line like this:

+   if ((TREE_CODE(type1)!=FUNCTION_TYPE))
+     return 1;

Instead it should look like this:

  if (TREE_CODE (type1) != FUNCTION_TYPE)
    return 1;


This fragment is not acceptable either:

+   /* Check for mismatched returntypes (cdecl vs stdcall). */ 
+   if ((0!=lookup_attribute (rtdstr, TYPE_ATTRIBUTES (type1))) !=
+       (0!=lookup_attribute (rtdstr, TYPE_ATTRIBUTES (type2)))
+   )


Lots of problems here.

First, I don't think you want a conditional like (x != 0) != (y != 0), for
this test I believe (x != y) is sufficent.

Second, when checking for zero, write it as (x != 0).  Or, in the case of
checking for a null tree pointer, (x != NULL_TREE).

When need to continue a conditional on a new line do so like this

if (cond1 ....
    && cond2 ....
    && (cond3 .....
        || cond4)))

Don't put the final close paren on a line by itself (there are some exceptions,
but this is not one of them.

And of course, you need to adhere to the whitespace conventions.  You must
always have a space between a binary operator and its operands.

After fixing these problems your patch looks like this:

  /* Check for mismatch of non-default calling convention. */
  char *rtdstr = TARGET_RTD ? "cdecl" : "stdcall";

  if (TREE_CODE (type1) != FUNCTION_TYPE)
    return 1;

  /* Check for mismatched return types (cdecl vs stdcall).  */
  if (lookup_attribute (rtdstr, TYPE_ATTRIBUTES (type1))
      != lookup_attribute (rtdstr, TYPE_ATTRIBUTES (type2)))
    return 0;
  return 1;


Which is much easier for folks which read GNU source all day to visually parse.

You should write ChangeLog entries in the style that is found in the ChangeLog
file.  In addition to the formatting of the ChangeLog, you are merely
indicating what change was made, not why it was made (the "why" belongs as
a comment in the code, not the ChangeLog)



I've fixed these problems this time and installed your patch; however, if you
submit any additional patches you will have to fix the formatting and
convention problems before we will install your patch.



jeff