[Fortran] Massive whitespace patch.

[Fortran] Massive whitespace patch.

[Steve Kargl]

[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]

The attached patch is a massive whitespace clean up of the
gfortran frontend *.c files.  It tries to provide a consistent,
uniform  style across all files.  Issues addressed in this 
patch include:

1) Indentation.  The indentation levels are in the following
   order: 2 spaces, 4 spaces, 6 spaces, tab, tab + 2 spaces,
   tab + 4 spaces,  tab + 6 spaces, 2 tabs, etc.

   There are exceptions to properly align multiline comments,
   function call, and rational operators.

2) Alignment.  I tried to enforce the style recommended in the
   GNU Coding Standard for multi-line rational operators in 
   if(), while(), etc.  Alignment was also enforced for the 
   arguments in a multiline function invocation.  Finally,
   comments were properly aligned in the surrounding code.

3) Whitespace.
   i)   Function prototypes with pointers have been updated to
        remove whitespace between the * and the symbol name.
   ii)  Warning and error messages that span multiple lines 
        have trailing spaces in the continues strings, eg

        gfc_error ("A trailing space here --> "
                   "preceeds this line");

   iii) Two blank lines follow the closing } in all functions.
   iv)  A blank line separates a function and comment that 
        describes the function.

4) Probably many things that I've forgotten.

I planned TO COMMIT this patch tomorrow.
 

2007-01-01  Steven G. Kargl  <kargl@gcc.gnu.org>

	* openmp.c: Massive whitespace cleanup.
	* interface.c: Ditto.
	* intrinsic.c: Ditto.
	* trans-expr.c: Ditto.
	* trans-array.c: Ditto.
	* symbol.c: Ditto.
	* decl.c: Ditto.
	* matchexp.c: Ditto.
	* dump-parse-tree.c: Ditto.
	* array.c: Ditto.
	* trans-openmp.c: Ditto.
	* error.c: Ditto.
	* data.c: Ditto.
	* trans-const.c: Ditto.
	* trans-stmt.c: Ditto.
	* expr.c: Ditto.
	* module.c: Ditto.
	* trans.c: Ditto.
	* trans-types.c: Ditto.
	* scanner.c: Ditto.
	* gfortranspec.c: Ditto.
	* bbt.c: Ditto.
	* io.c: Ditto.
	* resolve.c: Ditto.
	* f95-lang.c: Ditto.
	* st.c: Ditto.
	* iresolve.c: Ditto.
	* trans-io.c: Ditto.
	* trans-decl.c: Ditto.
	* match.c: Ditto.
	* arith.c: Ditto.
	* parse.c: Ditto.
	* check.c: Ditto.
	* dependency.c: Ditto.
	* convert.c: Ditto.
	* primary.c: Ditto.
	* trans-intrinsic.c: Ditto.
	* options.c: Ditto.
	* misc.c: Ditto.
	* simplify.c: Ditto.
-- 
Steve

[-- Attachment #2: zxc.diff.bz2 --]
[-- Type: application/octet-stream, Size: 160950 bytes --]

Re: [Fortran] Massive whitespace patch.

[Thomas Koenig]

On Mon, Jan 01, 2007 at 02:05:46PM -0800, Steve Kargl wrote:

> I planned TO COMMIT this patch tomorrow.

A few questions?

Did you regtest?

What will this do to backports to 4.2?

	Thomas

Re: [Fortran] Massive whitespace patch.

[Steve Kargl]

On Mon, Jan 01, 2007 at 11:34:33PM +0100, Thomas Koenig wrote:
> On Mon, Jan 01, 2007 at 02:05:46PM -0800, Steve Kargl wrote:
> 
> > I planned TO COMMIT this patch tomorrow.
> 
> A few questions?
> 
> Did you regtest?

Whoops.  Yes, it was regression tested on both
i386-*-freebsd and x86_64-*-freebsd.  There were
no regressions.

> What will this do to backports to 4.2?

I'm not sure what you mean.  This change does not
prevent anyone from back porting something in trunk
to 4.2.  In fact, gfortran should be following the
standard GCC guidelines with regard to back porting
to 4.2 (i.e, regression fixes only).

-- 
Steve

Re: [Fortran] Massive whitespace patch.

[Brooks Moses]

Steve Kargl wrote:
> On Mon, Jan 01, 2007 at 11:34:33PM +0100, Thomas Koenig wrote:
  >> What will this do to backports to 4.2?
> 
> I'm not sure what you mean.  This change does not
> prevent anyone from back porting something in trunk
> to 4.2.  In fact, gfortran should be following the
> standard GCC guidelines with regard to back porting
> to 4.2 (i.e, regression fixes only).

I think what Thomas is referring to is that any patchfile for 4.3 that 
touches a spot with changed whitespace will not apply to 4.2, and will 
thus require hand editing of the files to backport.  This makes 
backporting things substantially more laborious than the "apply the 
patch and regtest" process that it is now.

(My personal opinion is that this annoyance and extra work does not 
outweigh the value of doing a style cleanup.  I'm not sure about timing, 
though -- I can see arguments for why it might be useful to hold off 
until the 4.2 release, but I don't have any opinion one way or the other 
on their validity.)

- Brooks

Re: [Fortran] Massive whitespace patch.

[Steve Kargl]

On Mon, Jan 01, 2007 at 03:31:14PM -0800, Brooks Moses wrote:
> Steve Kargl wrote:
> >On Mon, Jan 01, 2007 at 11:34:33PM +0100, Thomas Koenig wrote:
>  >> What will this do to backports to 4.2?
> >
> >I'm not sure what you mean.  This change does not
> >prevent anyone from back porting something in trunk
> >to 4.2.  In fact, gfortran should be following the
> >standard GCC guidelines with regard to back porting
> >to 4.2 (i.e, regression fixes only).
> 
> I think what Thomas is referring to is that any patchfile for 4.3 that 
> touches a spot with changed whitespace will not apply to 4.2, and will 
> thus require hand editing of the files to backport.  This makes 
> backporting things substantially more laborious than the "apply the 
> patch and regtest" process that it is now.

I'm well aware of this complication.  But, I stand by my assertation
that the whitespace patch does not prevent back porting.  See 

http://en.wikipedia.org/wiki/Backport

> (My personal opinion is that this annoyance and extra work does not 
> outweigh the value of doing a style cleanup.  I'm not sure about timing, 
> though -- I can see arguments for why it might be useful to hold off 
> until the 4.2 release, but I don't have any opinion one way or the other 
> on their validity.)

The timing is such that I will not work on the ISO C Binding 
branch until after the whitespace patch is merged into branch.
This is perhaps the last major feature I will work on fixing.

-- 
Steve

Re: [Fortran] Massive whitespace patch.

[Mike Stump]

On Jan 1, 2007, at 3:31 PM, Brooks Moses wrote:
> Steve Kargl wrote:
>> On Mon, Jan 01, 2007 at 11:34:33PM +0100, Thomas Koenig wrote:
>  >> What will this do to backports to 4.2?

> I think what Thomas is referring to is that any patchfile for 4.3  
> that touches a spot with changed whitespace will not apply to 4.2,

If desired, whitespace can be also so fixed on the 4.2 branch, thus,  
the changes then port back with greater ease.  I think that is  
reasonable.

Re: [Fortran] Massive whitespace patch.

[Brooks Moses]

Steve Kargl wrote:
> On Mon, Jan 01, 2007 at 03:31:14PM -0800, Brooks Moses wrote:
>> I think what Thomas is referring to is that any patchfile for 4.3 that 
>> touches a spot with changed whitespace will not apply to 4.2, and will 
>> thus require hand editing of the files to backport.  This makes 
>> backporting things substantially more laborious than the "apply the 
>> patch and regtest" process that it is now.
> 
> I'm well aware of this complication.  But, I stand by my assertation
> that the whitespace patch does not prevent back porting.

And I am not disputing this assertion -- "substantially more laborious" 
is a very different thing from "impossible".  :)

I would be somewhat curious to know approximately how much of the 
codebase this ends up touching, on a percentage-of-lines-of-code basis; 
is this something like one line out of twenty, or more like every other 
line?

>> (My personal opinion is that this annoyance and extra work does not 
>> outweigh the value of doing a style cleanup.  I'm not sure about timing, 
>> though -- I can see arguments for why it might be useful to hold off 
>> until the 4.2 release, but I don't have any opinion one way or the other 
>> on their validity.)
> 
> The timing is such that I will not work on the ISO C Binding 
> branch until after the whitespace patch is merged into branch.

In which case, I'd agree that that's by far the most significant reason 
that I see for committing this at some particular point or another.

On a different note, how much of this work did you do manually, and how 
much by automated search-and-replace stuff?  If there was a lot of the 
latter, I think it would be helpful if you could share some of the 
techniques, because I suspect that merging this into various other 
overall-GCC development branches is going to be "interesting" if they've 
touched a fair bit of Fortran code.  (I don't know what the situation is 
what that, overall; I can say that what I'm helping with on the new 
CALL_EXPR representation from the LTO branch has some 35kb of Fortran 
patches.)

- Brooks

Re: [Fortran] Massive whitespace patch.

[Steve Kargl]

On Mon, Jan 01, 2007 at 03:58:44PM -0800, Mike Stump wrote:
> On Jan 1, 2007, at 3:31 PM, Brooks Moses wrote:
> >Steve Kargl wrote:
> >>On Mon, Jan 01, 2007 at 11:34:33PM +0100, Thomas Koenig wrote:
> > >> What will this do to backports to 4.2?
> 
> >I think what Thomas is referring to is that any patchfile for 4.3  
> >that touches a spot with changed whitespace will not apply to 4.2,
> 
> If desired, whitespace can be also so fixed on the 4.2 branch, thus,  
> the changes then port back with greater ease.  I think that is  
> reasonable.

Mike, I literally visually scanned every line of code to develop
this patch.  The gfortran frontend code was a potpourri of styles.
There were no automated tools to clean this up.  The OpenMp files
were the cleanest in the patch, and yet I add blanked line to 
make it conform to the rest of gfortran (apologies to Jakub, rth,
and Diego).

-- 
Steve

Re: [Fortran] Massive whitespace patch.

[Mike Stump]

On Jan 1, 2007, at 4:55 PM, Steve Kargl wrote:
> Mike, I literally visually scanned every line of code to develop
> this patch.  The gfortran frontend code was a potpourri of styles.
> There were no automated tools to clean this up.

Ah, I do this type of thing from time to time and I use emacs to do  
most of the work, though, I do eyeball tons of code to ensure that  
everything so changed is ok.

Also, the other technique would be to just force apply the patch to  
the 4.2 branch and check in the result.  Those hunks that are  
similar, get patched, those that don't, well, let's just say, they  
aren't going to easily support backporting anyway.

Re: [Fortran] Massive whitespace patch.

[Brooks Moses]

Steve Kargl wrote, on 2007/01/01:
> The attached patch is a massive whitespace clean up of the
> gfortran frontend *.c files.  It tries to provide a consistent,
> uniform  style across all files.  Issues addressed in this 
> patch include:
[...]
> I planned TO COMMIT this patch tomorrow.

In case it wasn't clear, my questions about this have definitely been 
answered to my satisfaction, and it looks like Thomas's were answered as 
well.  So, I'm hoping you'll go ahead and commit it quickly, so I don't 
have to worry about bit-rotting it if I need to poke in some files that 
it changes a lot.  :)

Mike's suggestion of force-applying the patch file to 4.2 and just 
leaving out whatever doesn't apply sounds like a good one to me, for 
dealing with that, but IMO deciding whether to do that certainly 
shouldn't block applying this to trunk.

- Brooks