Re: Allow embedded timestamps by C/C++ macros to be set externally (3)

[Eduard Sanou <eduardsanou at openmailbox dot org>]

Hi,

I've worked on applying the changes that have been commented in several
messages from this thread.  Find the patch and changelog attached.

On 16-04-29 09:17:44, Jakub Jelinek wrote:
> First of all, using error instead of fatal_error achieves just that too,
> except that it allows also detecting other errors in the source.
> fatal_error is meant for cases where there is no way to go forward with the
> compilation, while here exists a perfectly reasonable way to go forward (assume
> the env var is not set, use a hardcoded timestamp, ...).

I've changed the use of fatal_error to error_at.  Now the parsing will
return -1 if there's an error, and gcc will be able to output the line
where the __DATE__ or __TIME__ macro was used that caused the error.

> Doing this on the gcc/ side is of course reasonable, but can be done through
> callbacks, libcpp already has lots of other callbacks into the gcc/ code,
> look for e.g. cpp_get_callbacks in gcc/c-family/* and in libcpp/ for
> corresponding code.

I've added a callback that allows parsing the SOURCE_DATE_EPOCH env var
right at the point where the date macros are expanded.  I've removed
some uneeded stuff from my previous patch.

> Also, as a follow-up, guess the driver should set this
> env var for the -fcompare-debug case if not already set, to something that
> matches the current date, so that __TIME__ macros expands the same in
> between both compilations, even when they don't compile both in the same
> second.

I've also patched the driver to set the SOURCE_DATE_EPOCH env var to the
current date when the -fcompare-debug is set.  I've tested this building
llvm with such flag.  
Markus could you verify that the patch indeed fixes the issue?


Martin Sebor wrote:
> I'm sorry I'm a little late but I have a couple of minor comments
> on the patch:

Thanks for the comments :)

> In most texts (e.g. the C and POSIX standards), the name of
> an environment variable doesn't include the dollar sign.  In
> other diagnostic messages GCC doesn't print one.  I suggest
> to follow the established practice and remove the dollar sign
> from this error message as well.

Change applied.

> I would also suggest to issue a single generic error message
> explaining what the valid value of the variable is instead of
> trying to describe what's wrong with it, for example as follows
> (note also the hyphen in "non-negative" which is the prevalent
> style used by other GCC messages and GNU documentation).
> 
>   "environment variable SOURCE_DATE_EPOCH must expand to a non-
>   negative integer less than or equal to %qlli", LLONG_MAX

Applied as well.

> The +%s option to the date command is a non-standard extension
> that's not universally available.  To avoid confusing users on
> systems that don't support it I would suggest to either avoid
> mentioning or to clarify that it's a Linux command.

I've added a comment to clarify this fact in the documentation.

Cheers,
Dhole

Attachment: ChangeLog
Description: Text document

Attachment: gcc-SOURCE_DATE_EPOCH-patch-2016_05_05.diff.txt
Description: Text document

Attachment: signature.asc
Description: PGP signature