OSGeo / grass

GRASS GIS - free and open-source geospatial processing engine
https://grass.osgeo.org
Other
847 stars 308 forks source link

[Bug] Bulk re-indentation leads to errors #1630

Closed neteler closed 1 year ago

neteler commented 3 years ago

Describe the bug

It may be a good idea to run a bulk re-indentation of all C/C++/h files before releasing GRASS GIS 8.0.0 (similar to the Python reformatting already done).

The problem is that (in a local dry-run) errors occur:

utils/grass_indent_ALL.sh
Indenting *.c and *.h...
indent: ./vector/v.in.ogr/main.c:1457: Error:Stmt nesting error.
indent: ./vector/v.in.ogr/main.c:1779: Error:Stmt nesting error.
indent: ./vector/v.in.ogr/main.c:2042: Error:Stmt nesting error.
indent: ./imagery/i.eb.hsebal01/main.c:547: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./imagery/i.eb.hsebal01/main.c:557: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./imagery/i.evapo.pm/functions.c:161: Error:Unexpected end of file
indent: ./include/grass/iostream/empq_adaptive_impl.h:188: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./include/grass/iostream/minmaxheap.h:198: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./lib/vector/dglib/misc-template.c:574: Error:Stmt nesting error.
indent: ./lib/vector/dglib/misc-template.c:655: Error:Unmatched 'else'
indent: ./lib/vector/dglib/misc-template.c:684: Error:Stmt nesting error.
indent: ./raster/r.terraflow/3scan.h:123: Warning:old style assignment ambiguity in "=*". Assuming "= *"

indent: ./raster/r.terraflow/types.h:33: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./raster/r.terraflow/types.h:75: Warning:old style assignment ambiguity in "=-". Assuming "= -"

cmp: EOF on ./display/d.vect/plot.h after byte 782
cmp: EOF on ./vector/v.mkgrid/local_proto.h after byte 279
cmp: EOF on ./ps/ps.map/decorate.h after byte 462
cmp: EOF on ./lib/display/clip.h after byte 451
cmp: EOF on ./lib/display/window.c after byte 594
cmp: EOF on ./lib/htmldriver/box.c after byte 211
cmp: EOF on ./lib/htmldriver/draw.c after byte 427
cmp: EOF on ./lib/segment/local_proto.h after byte 631
cmp: EOF on ./lib/driver/move.c after byte 115
cmp: EOF on ./lib/calc/calc.c after byte 1143
cmp: EOF on ./lib/calc/function.c after byte 1839
cmp: EOF on ./raster/r.li/r.li.daemon/ipc.h after byte 2568
cmp: EOF on ./raster/r.clump/rclist.h after byte 310

While a bulk re-indentation will cause major troubles with open pull requests (which would require reformatting/rebasing) it might be well worth to address the errors/warnings above anyway.

nilason commented 3 years ago

At this very moment I'm working on a PR on grass_indent_ALL :-) , and I encountered the same "indent" warnings (but not the "cmp" warnings).

grass_indent_ALL.sh
Indenting *.c and *.h...
indent: ./imagery/i.eb.hsebal01/main.c:547: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./imagery/i.eb.hsebal01/main.c:557: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./imagery/i.evapo.pm/functions.c:161: Error:Unexpected end of file
indent: ./include/grass/iostream/minmaxheap.h:198: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./include/grass/iostream/empq_adaptive_impl.h:188: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./lib/vector/dglib/misc-template.c:574: Error:Stmt nesting error.
indent: ./lib/vector/dglib/misc-template.c:655: Error:Unmatched 'else'
indent: ./lib/vector/dglib/misc-template.c:684: Error:Stmt nesting error.
indent: ./raster/r.terraflow/types.h:33: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./raster/r.terraflow/types.h:75: Warning:old style assignment ambiguity in "=-". Assuming "= -"

indent: ./raster/r.terraflow/3scan.h:123: Warning:old style assignment ambiguity in "=*". Assuming "= *"

indent: ./vector/v.in.ogr/main.c:1457: Error:Stmt nesting error.
indent: ./vector/v.in.ogr/main.c:1779: Error:Stmt nesting error.
indent: ./vector/v.in.ogr/main.c:2042: Error:Stmt nesting error.

Looking into the specifics on these warnings, I concluded they may be ignored. The handling of "old style assignment ambiguity" is correct (as far as I can tell) and the nesting errors are caused by preprocessor macros like #if XXX clauses which GNU Indent apparently can't handle.

nilason commented 3 years ago

From WP on K&R C:

Compound_ assignment operators of the form =op (such as =-) were changed to the form op= (that is, -=) to remove the semantic ambiguity created by constructs such as i=-10, which had been interpreted as i =- 10 (decrement i by 10) instead of the possibly intended i = -10 (let i be −10).

wenzeslaus commented 3 years ago

Then I would guess -= and *= is what the code is meant to do, not = *. I didn't look to the source code, though.

nilason commented 3 years ago

Then I would guess -= and *= is what the code is meant to do, not = *. I didn't look to the source code, though.

I did look at source (and when needed went back in history), which is why I dare to draw the stated conclusion. Also, I assume the compilers interpret the code as Indent (= * and = -), considering =- was abandoned with K&R C. That being said, will not hurt to take another look at it. The more eyes the merrier.

wenzeslaus commented 3 years ago

@nilason I checked the code and you are right. These are all correct, so no problem formatting them. I didn't realize some are in C++ where =op was never an operator (at least I hope so) and they all are =- and =* not anything else.

I looked at some of the #if related issues. There is the rule that if it is too complicated for the machine, it is no good for a human either, so maybe the code should be cleaned up. In some cases, I definitively side with GNU indent as I'm not able to match the correct braces together.

wenzeslaus commented 2 years ago

With #2579 and #2580, the following issues are remaining:

indent: ./vector/v.in.ogr/main.c:1457: Error:Stmt nesting error.
indent: ./vector/v.in.ogr/main.c:1779: Error:Stmt nesting error.
indent: ./vector/v.in.ogr/main.c:2042: Error:Stmt nesting error.
indent: ./lib/vector/dglib/misc-template.c:574: Error:Stmt nesting error.
indent: ./lib/vector/dglib/misc-template.c:655: Error:Unmatched 'else'
indent: ./lib/vector/dglib/misc-template.c:684: Error:Stmt nesting error.
neteler commented 2 years ago

Is anything left to be done here?

wenzeslaus commented 2 years ago

The PRs are not merged and I don't know about any PRs addressing the remaining issues above, so yes. These need to be solved (in code):

indent: ./vector/v.in.ogr/main.c:1457: Error:Stmt nesting error. indent: ./vector/v.in.ogr/main.c:1779: Error:Stmt nesting error. indent: ./vector/v.in.ogr/main.c:2042: Error:Stmt nesting error. indent: ./lib/vector/dglib/misc-template.c:574: Error:Stmt nesting error. indent: ./lib/vector/dglib/misc-template.c:655: Error:Unmatched 'else' indent: ./lib/vector/dglib/misc-template.c:684: Error:Stmt nesting error.

It seems it is related to complex preprocessor if constructions.

nilason commented 1 year ago

Solved/overridden by #2709 - #2715.