Обсуждение: 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. -- 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)
>
> 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)
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
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
>
> 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)
> > 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)