I have written a script to remove braces around single statements, if
the statement is only one line in length.

The macro fixup context diff was 1,200 lines, and this diff is 12k
lines.

Hope no one is sitting on patches.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

Mon, 15 Jun 1998 15:16:02 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
removal of braces
> 
> I have written a script to remove braces around single statements, if
> the statement is only one line in length.
> 
> The macro fixup context diff was 1,200 lines, and this diff is 12k
> lines.
> 
> Hope no one is sitting on patches.

They had things like:

	if (test != 0)
		macro;

while the macro was:

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

Of course, only the stmt1 is conditional.  The rest are always executed.
I am sure there were some bugs fixed by this cleanup.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

Mon, 15 Jun 1998 16:55:17 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] removal of braces
Bruce Momjian chortles ominiously:
> I have written a script to remove braces around single statements, if
> the statement is only one line in length.
> 
> The macro fixup context diff was 1,200 lines, and this diff is 12k
> lines.
> 
> Hope no one is sitting on patches.

Is this trip necessary? While I am a strong believer in aesthetics when
it comes to code (make it pretty first, making pretty code work is easy),
I am not sure I support wholesale changes (12,000 lines of diff) for
the sake of purely cosmetic issues.

It is somewhat costly to the developers as we will all have to pull a complete
new source tree from CVS.

It is also somewhat risky. Suppose the script makes an error some
where due to a tricky macro or suchlike. If this is not in something that
gets checked by the regression test how likely are we to find it?

And, for those of us contemplating larger projects where we might change a
large number of files over a period of weeks or months, it presents a
really scary merge problem.

That said, if you get my patch in before you whack the braces, I don't have
anything right now that would be harmed.

-dg

David Gould            dg@illustra.com           510.628.3783 or 510.305.9468 
Informix Software  (No, really)         300 Lakeside Drive  Oakland, CA 94612
"Don't worry about people stealing your ideas.  If your ideas are any
 good, you'll have to ram them down people's throats." -- Howard Aiken

Mon, 15 Jun 1998 13:59:58 -0700 (PDT)
dg@illustra.com (David Gould)
Re: [HACKERS] removal of braces
Bruce Momjian <maillist@candle.pha.pa.us> writes:
> I am sure there were some bugs fixed by this cleanup.

And some introduced, most likely.  Just how much does your script know
about C syntax, macros, etc?

I concur with David Gould that this is a risky, expensive exercise
with very little potential gain.

If you were able to identify any spots with the sort of macro-induced
bug you illustrated, great!  Fix 'em by hand.  But I doubt that a
12000-line wholesale modification is worth the trouble.  Heck, half
the code I've looked at in pgsql doesn't even conform to indentation/
tab-width conventions ... and *those* we have well-tested tools to fix.

			regards, tom lane

Mon, 15 Jun 1998 17:24:45 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] removal of braces 
> 
> Bruce Momjian chortles ominiously:
> > I have written a script to remove braces around single statements, if
> > the statement is only one line in length.
> > 
> > The macro fixup context diff was 1,200 lines, and this diff is 12k
> > lines.
> > 
> > Hope no one is sitting on patches.
> 
> Is this trip necessary? While I am a strong believer in aesthetics when
> it comes to code (make it pretty first, making pretty code work is easy),
> I am not sure I support wholesale changes (12,000 lines of diff) for
> the sake of purely cosmetic issues.
> 
> It is somewhat costly to the developers as we will all have to pull a complete
> new source tree from CVS.

Yes, it is a tradeoff in making so much of a change, but I reviewed the
code, and patch file, and it looked good.  I thought it was broken
because I could not recompile after it, but then realized the macros
were broken.

> It is also somewhat risky. Suppose the script makes an error some
> where due to a tricky macro or suchlike. If this is not in something that
> gets checked by the regression test how likely are we to find it?

If it is broken, we back out the changes.

> 
> And, for those of us contemplating larger projects where we might change a
> large number of files over a period of weeks or months, it presents a
> really scary merge problem.

The code was very conservative:

	awk '	{	line3 = $0;  /* remove un-needed braces around single statements */
			if (skips > 0)
				skips--;
			if (line1 ~ "		*{$" &&
			    line2 ~ "		*[^;{}]*;$" &&
			    line3 ~ "		*}$")
			{
				print line2;
				line1 = "";
				line2 = "";
				line3 = "";
				skips = 3;
			}
			else
	 			if (skips == 0 && NR >= 3)
					print line1;
			line1 = line2;
			line2 = line3;
			line3 = "";
		}
		END {
			if (skips <= 1)
				print line1;
			if (skips <= 2)
				print line2;
	}'


> 
> That said, if you get my patch in before you whack the braces, I don't have
> anything right now that would be harmed.


-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

Tue, 16 Jun 1998 03:22:07 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] removal of braces
> 
> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> > I am sure there were some bugs fixed by this cleanup.
> 
> And some introduced, most likely.  Just how much does your script know
> about C syntax, macros, etc?

I have posted the patch to the list.

> 
> I concur with David Gould that this is a risky, expensive exercise
> with very little potential gain.
> 
> If you were able to identify any spots with the sort of macro-induced
> bug you illustrated, great!  Fix 'em by hand.  But I doubt that a
> 12000-line wholesale modification is worth the trouble.  Heck, half
> the code I've looked at in pgsql doesn't even conform to indentation/
> tab-width conventions ... and *those* we have well-tested tools to fix.

That is more work than fixing all the macros.  I think 98% should be
properly formatted for 4-space tabs.  I run pgindent just before every
beta period, and have added the brace removal code to pgindent.

I got tired of removing them as I saw them.  Now, we have a standard.


-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

Tue, 16 Jun 1998 03:23:51 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] removal of braces