[PATCH] Write dependency information (-M*) even if there are errors

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] Write dependency information (-M*) even if there are errors

Boris Kolpackov-2
Hi,

Currently GCC does not write extracted header dependency information
if there are errors. However, this can be useful when dealing with
outdated generated headers that trigger errors which would have been
resolved if we could update it. A concrete example in our case is a
version check with #error.

The included (trivial) patch changes this behavior. Note also that
this is how Clang already behaves. I've tested the patch in build2
and everything works well (i.e., no invalid dependency output in the
face of various preprocessor errors such as #error, stray #else, etc).

While I don't foresee any backwards-compatibility issues with such
an unconditional change (after all, the compiler still exists with
an error status), if there are concerns, I could re-do it via an
option (e.g., -ME, analogous to -MG).

P.S. I have the paperwork necessary to contribute on file with FSF.

Thanks,
Boris

deps-on-error.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Write dependency information (-M*) even if there are errors

Segher Boessenkool
Hi!

Patches should go to gcc-patches.

Just a trivial remark:

> --- gcc/c-family/c-opts.c (revision 250514)
> +++ gcc/c-family/c-opts.c (working copy)
> @@ -1152,8 +1152,11 @@
>  {
>    FILE *deps_stream = NULL;
>  
> -  /* Don't write the deps file if there are errors.  */
> -  if (cpp_opts->deps.style != DEPS_NONE && !seen_error ())
> +  /* Note that we write the dependencies even if there are errors. This is
> +     useful for handling outdated generated headers that now trigger errors
> +     (for example, with #error) that would be resolved by re-generating
> +     them. In a sense this complements -MG. */

Two spaces after a full stop (all three times).


Segher
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Write dependency information (-M*) even if there are errors

Jeff Law
In reply to this post by Boris Kolpackov-2
On 08/06/2017 01:59 AM, Boris Kolpackov wrote:

> Hi,
>
> Currently GCC does not write extracted header dependency information
> if there are errors. However, this can be useful when dealing with
> outdated generated headers that trigger errors which would have been
> resolved if we could update it. A concrete example in our case is a
> version check with #error.
>
> The included (trivial) patch changes this behavior. Note also that
> this is how Clang already behaves. I've tested the patch in build2
> and everything works well (i.e., no invalid dependency output in the
> face of various preprocessor errors such as #error, stray #else, etc).
>
> While I don't foresee any backwards-compatibility issues with such
> an unconditional change (after all, the compiler still exists with
> an error status), if there are concerns, I could re-do it via an
> option (e.g., -ME, analogous to -MG).
>
> P.S. I have the paperwork necessary to contribute on file with FSF.
This directly reverts part of Joseph's changes from 2009.   I'd like to
hear from him on this change.



commit 7f5f395354b35ab7f472d03dbcce1301ac4f8257
Author: jsm28 <jsm28@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Sun Mar 29 22:56:07 2009 +0000

            PR preprocessor/34695

[ ... ]
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@145263
138bc75d-0d04-0410-961f-82ee72b054a4

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Write dependency information (-M*) even if there are errors

Joseph Myers
On Wed, 9 Aug 2017, Jeff Law wrote:

> This directly reverts part of Joseph's changes from 2009.   I'd like to
> hear from him on this change.

The point of those changes was to make cpplib diagnostics use the
compiler's diagnostic machinery rather than a separate set of diagnostic
machinery in cpplib.  The description, regarding dependency information
generation, in the original patch description
<https://gcc.gnu.org/ml/gcc-patches/2009-02/msg00491.html>, is "the code
in cpplib that checked for errors before deciding whether to write
dependency output no longer does so (instead, the compiler has the same
check, but this time based on whether there were any errors at all,
whether compiler or preprocessor)".

That is, that patch wasn't meant to make any change to how errors affect
dependency generation beyond causing compiler errors to be handled the
same as preprocessor errors.

I suppose a question for the present proposal would be making sure any
dependencies generated in this case do not include dependencies on files
that don't exist (so #include "some-misspelling.h" doesn't create any sort
of dependency on such a header).

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Write dependency information (-M*) even if there are errors

Boris Kolpackov-2
Joseph Myers <[hidden email]> writes:

> I suppose a question for the present proposal would be making sure any
> dependencies generated in this case do not include dependencies on files
> that don't exist (so #include "some-misspelling.h" doesn't create any sort
> of dependency on such a header).

Good point. I've tested this and I believe everything is in order:
unless -MG is specified, a non-existent header is treated as a fatal
error so we don't even get to writing the dependency info. And if -MG
is specified, then there is no error and we get the missing header in
the dependency output, as requested.

Boris
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Write dependency information (-M*) even if there are errors

Boris Kolpackov-2
In reply to this post by Segher Boessenkool
Segher Boessenkool <[hidden email]> writes:

> Patches should go to gcc-patches.

Ok, will keep in mind for future (seeing that we have a discussion
already it probably doesn't make sense to move this patch).

 
> Two spaces after a full stop (all three times).

Fixed, new revision included.


Thanks,
Boris

deps-on-error.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Write dependency information (-M*) even if there are errors

Segher Boessenkool
Hi Boris,

On Sat, Aug 12, 2017 at 05:53:44PM +0200, Boris Kolpackov wrote:
> Segher Boessenkool <[hidden email]> writes:
>
> > Patches should go to gcc-patches.
>
> Ok, will keep in mind for future (seeing that we have a discussion
> already it probably doesn't make sense to move this patch).

Please do move it; gcc-patches is where people look for patches, not
here (say, a year from now, when someone want to look up history of
this patch).


Segher
Loading...