Re: [PATCHv3 1/6] Add missing includes and forward declares

Elijah Newren wrote:

I didn't want to repeat that description in all 6 patches, since all
six came from that, so I put it in the cover letter.  Since patch #1
has most that changes though, I guess it makes sense to include it at
least in that one?

Yes, that sounds sensible to me.

[...]

On Tue, Aug 14, 2018 at 10:10 PM Jonathan Nieder [off-list ref] wrote:

quoted

Is there an easy way to review it?  (Keep in mind that I'm super lazy.
;-))

I guess I could send you my hacky python script that loops through the
top-level header files and creates the dummy two-line c file, and you
could inspect it and run it.  But that only verifies that it compiles,
not that the changes I choose are "correct".

I suppose leaving it as an exercise to the reviewer is not terrible.
Maybe some reviewer will be inspired to make a test tool we can check
in, or configuration for an existing tool like iwyu.

[...]

quoted

enums are of unknown size, so forward declarations don't work for
them.  See bb/pedantic for some examples.

structs are also of unknown size; the size is irrelevant when the
function signature merely uses a pointer to the struct or enum.  The
enum forward declaration fixes a compilation bug.

My rationale may miss the point but the standard and some real compilers
don't like this, unfortunately.

For structs, having an incomplete type is fine, but for enums we need
the full definition.  E.g. C99 sayeth (in section 6.7.2.3 "tags")

	A type specifier of the form

		enum identifier

	without an enumerator list shall only appear after the type it
	specifies is complete.

[...]

quoted

quoted

--- a/commit-graph.h
+++ b/commit-graph.h

@@ -4,6 +4,7 @@
 #include "git-compat-util.h"
 #include "repository.h"
 #include "string-list.h"
+#include "cache.h"

We can skip the #include of git-compat-util.h since all .c files
include it.

Good point.  Should I go through and remove all the inclusions of
git-compat-util.h in header files?

It's orthogonal to this series but might be a good change.

[...]

quoted

quoted

--- a/pathspec.h
+++ b/pathspec.h

@@ -1,6 +1,11 @@
 #ifndef PATHSPEC_H
 #define PATHSPEC_H

+#include "string.h"
+#include "strings.h"

What are these headers?

The original patch[1] had explanations of why I added them:

+#include "string.h"   /* For str[n]cmp */
+#include "strings.h"  /* For str[n]casecmp */

Ah.  Please remove these #includes: they're part of the standard
library that we get implicitly via git-compat-util.h.

I was tripped up because they were in quotes instead of angle
brackets.

Thanks,
Jonathan