[ping][patch] Proposal to add new warning -Wdate-time

Clang Frontend
#1

Hi,
    I am pinging for this patch, which proposes to add new warning -Wdate-time:
http://clang-developers.42468.n3.nabble.com/proposal-to-add-new-warning-Wdate-time-td4038953.html
I would be grateful if someone would review it for me.

Thanks and Regards,
Prathamesh

#2

Great thing to add. I agree it is nice to have this in Clang.
Reproducible builds are a goal in Debian.
About the patch itself, it would be nice to have test for it but besides
that, it looks good.

Sylvestre

#3

Rather than the warning, it would be nice to have some analogue of -frandom-seed= that would allow explicitly setting the timestamp to be used. When you want reproduceable builds, you should set this, just as you set -frandom-seed=.

David

#4

Mostly to play devil's advocate...

What would this warning gain us over simply using grep? The use of the macro is purely a textual property, there's no real parsing required.

If we do accept such a warning, having a more general category for reproducibility problems might be a better approach. Time and Date are one example, but there may be others.

Philip

#5

Mostly to play devil's advocate...

What would this warning gain us over simply using grep? The use of the
macro is purely a textual property, there's no real parsing required.

Although not realistic, the warning won't be issued if __DATE__/__TIME__,
gets undefined or redefined.

#6

> Mostly to play devil's advocate...
>
> What would this warning gain us over simply using grep? The use of the
> macro is purely a textual property, there's no real parsing required.
Although not realistic, the warning won't be issued if __DATE__/__TIME__,
gets undefined or redefined.

Devil's advocate again: '-D__DATE__=_Pragma("GCC warning(\"foo\")")' or
just '-U__DATE__' ?

> If we do accept such a warning, having a more general category for
> reproducibility problems might be a better approach. Time and Date are
one
> example, but there may be others.

I don't see that this is an either/or choice. We can add -Wdate-time now
(or whatever we want to call it) and add it to a -Wreproducibility group
later, when we have other things to put in that group.

#7

It would give a little more gcc compatibility, as -Wdate-time was added to gcc 4.9 (as Prathamesh said at the beginning of this thread).

Cheers,

Sean

#8

Nobody actually likes having to run pre-commit hooks written with grep.
Warnings are easier to sort out.

+1 for -Wdate-time

#9

Totally missed this thread and ended up implementing -Wdate-time earlier today as a means to validate our new system header warning suppression semantics.

Please see r210511 + r210512 for the approach taken. Apologies to Prathamesh whose original patch appears to have been on the right track (modulo IdentifierInfo comparisons and missing __TIMESTAMP__ case).

Alp.