Re: [PATCH v3 6/8] git-compat-util.h: move warning infra to prepare for PCHs

[Toon Claes <toon@iotcl.com>]

Patrick Steinhardt <ps@pks.im> writes:

> The "git-compat-util.h" header is supposed to be the first header
> included by every code compilation unit. As such, a subsequent commit
> will start to precompile this header to speed up compilation of Git.
>
> This will cause an issue though with the way that we have set up the
> "-Wsign-compare" warnings. It is expected that any compilation unit that
> fails with that compiler warning sets `DISABLE_SIGN_COMPARE_WARNINGS`
> before including "git-compat-util.h". If so, we'll disable the warning
> right away via a compiler pragma.
>
> But with precompiled headers we do not know ahead of time whether the
> code unit wants to disable those warnings, and thus we'll have to
> precompile the header without defining `DISABLE_SIGN_COMPARE_WARNINGS`.
> But as the pragma statement is wrapped by our include guards, the second
> include of that file will not have the desired effect of disabling the
> warnings anymore.
>
> We could fix this issue by declaring a new macro that compilation units
> are expected to invoke after having included the file. In retrospect,
> that would have been the better way to handle this as it allows for
> more flexibility: we could for example toggle the warning for specific
> code blocks, only. But changing this now would require a bunch of
> changes, and the churn feels excessive for what we gain.
>
> Instead, prepare for the precompiled headers by moving the code outside
> of the include guards.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  git-compat-util.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index bebcf9f698..4b4ea2498f 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -34,10 +34,6 @@ struct strbuf;
>  #  define DISABLE_WARNING(warning)
>  #endif
>  
> -#ifdef DISABLE_SIGN_COMPARE_WARNINGS
> -DISABLE_WARNING(-Wsign-compare)
> -#endif
> -
>  #undef FLEX_ARRAY
>  #define FLEX_ARRAY /* empty - weather balloon to require C99 FAM */
>  
> @@ -1099,3 +1095,7 @@ extern int not_supposed_to_survive;
>  #endif /* CHECK_ASSERTION_SIDE_EFFECTS */
>  
>  #endif
> +
> +#ifdef DISABLE_SIGN_COMPARE_WARNINGS
> +DISABLE_WARNING(-Wsign-compare)
> +#endif

Okay, so with all patches applied, when a .c file is compiled,
tools/precompiled.h is included as the first one. That one includes
git-compat-util.h and processes everything inside the include guards.
Then it starts processing the contents of that files and that file might
#define DISABLE_SIGN_COMPARE_WARNINGS. Usually git-compat-util.h is then
included again, but thanks to the include guards, most of it is ignored,
except for this last bit.

Okay, makes sense.

-- 
Cheers,
Toon