[PATCH v6 0/4] Add include guard checker and reformatter

Tom de Vries tdevries@suse.de
Fri Dec 13 15:03:42 GMT 2024


On 12/13/24 15:39, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
> 
> Tom> Here's v6 of the series.  I believe it addresses all the review
> Tom> comments.
> 
> Tom> I reordered the series little, since it makes more sense to enable
> Tom> pre-commit as the last step.
> 
> I've updated this series locally -- in particular I changed the script
> to allow certain headers to be excluded from the check.  This was needed
> to bypass gdbsupport/unordered_dense.h, which doesn't follow local
> header rules because it isn't really maintained as part of gdb.

I tried to apply the series, but it doesn't apply cleanly anymore, not 
surprising for a patch set touching this many files.

Adding the "excluding certain headers" functionality makes sense to me.

> One oddity I've thought of is that this script means that pure header
> file moves aren't really possible any more.  For example, a hypothetical
> "gdb/x.h" will use a header guard of "GDB_X_H"; but if it is then moved
> to gdbsupport, the guard will change to "GDBSUPPORT_X_H" -- requiring an
> edit during the move.
> 
> I personally don't care about this but I figured I should point it out.
> 

Indeed, but I think this is fine.

> One option would be to not use the directory name in the guard, but
> instead always use the same prefix, like "GDB".  This might imply adding
> a check to ensure that there aren't any clashes.
> 

I prefer keeping the directory name in there.  Anyway, if we change our 
mind about that at some point, changing it is easy now :)

LGTM.

Reviewed-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom

> Let me know what you think.
> 
> Tom



More information about the Gdb-patches mailing list