[Bug c++/96570] New: Warnings desired for time_t to int coversions

[Bug c++/96570] New: Warnings desired for time_t to int coversions

[terra at gnome dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96570

            Bug ID: 96570
           Summary: Warnings desired for time_t to int coversions
           Product: gcc
           Version: 10.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: terra at gnome dot org
  Target Milestone: ---

This is a wishlist item.  It's filed for C++ but would apply to C too.

It would be useful to have some mechanism to cause warnings to be emitted at
compile-time when values of type time_t are cut down in size.  For example:

int now = time(nullptr); // Not good!

int now = int(time(nullptr)); // Not good!
// (This one occurs not only when someone wanted the compiler to shut up,
// but also occurs when using casts to resolve overload resolution.)


void foo(int);
...
time_t now = time(nullptr);
foo(now); // Not good

Note: this is desired for explicit casts too.  Hence -Wconversion in its
current form is not what I am looking for.  I am not sure how, if at all, the
warnings should be suppressed, but a cast via a 64-bit type,
int(int64_t(time(null))), is an option.

[Bug c++/96570] Warnings desired for time_t to int coversions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96570

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Blocks|                            |87403

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to M Welinder from comment #0)
> Note: this is desired for explicit casts too.

Why? If somebody wants to be explicitly stupid, that's their prerogative.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87403
[Bug 87403] [Meta-bug] Issues that suggest a new warning

[Bug c++/96570] Warnings desired for time_t to int coversions

[terra at gnome dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96570

--- Comment #2 from M Welinder <terra at gnome dot org> ---
> Why? If somebody wants to be explicitly stupid, that's their prerogative.

I agree with the second sentence.

However, casts are not a clear indication that somebody wants to be explicitly
stupid, at least not in C++.  If you were looking for such an indication, my
int(int64_t(...)) suggestion is probably closer.

Casts occur also in (e.g.) overload resolution and entirely too often in
template soup.  And in macros too, I guess.

The goal here is to find time handling bugs.  We are by definition talking
about code that should not have been written that way in the first place.

[Bug c++/96570] Warnings desired for time_t to int coversions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96570

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to M Welinder from comment #2)
> Casts occur also in (e.g.) overload resolution and entirely too often in
> template soup.  And in macros too, I guess.

Explicit casts don't, and that's what I was questioning.

-Wconversion will warn about implicit conversions. What explicit conversions
happen inadvertently?

[Bug c++/96570] Warnings desired for time_t to int coversions

[terra at gnome dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96570

--- Comment #4 from M Welinder <terra at gnome dot org> ---
> Explicit casts don't, and that's what I was questioning.

They most certainly do.  That's an empirical statement from having gone over a
fairly large code base.  It is not a statement that they should occur there and
there is likely nothing "inadvertent[ly]" about their presence.  "Mistaken" and
"ill-advised" are probably better descriptions, but the reasons have long since
been forgotten.  I.e., the code is buggy.

Look, you are being a bit defensive here which is strange as no-one is
attacking you.  Please try looking at the goal:

*** The goal here is to find time handling bugs.

Is that a worthy goal for gcc?

I will assert that there is a lot of buggy time handling code out there and
that fixing it will receive more and more attention over the next 15 years.

The compiler may or may not be the right tool to help find and fix these, but
gcc has in the past taken it upon itself to warn about other classes of
likely-wrong code and it is fairly well positioned to do so given its access to
a parse tree and type information.

[Bug c++/96570] Warnings desired for time_t to int coversions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96570

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to M Welinder from comment #4)
> > Explicit casts don't, and that's what I was questioning.
> 
> They most certainly do.

I think I understand what you mean now, cases like:

int i = std::max(t, int(time(nullptr)));

?

That's more compelling than:

int now = int(time(nullptr)); // Not good!

There seems absolutely no reason to warn here. The user clearly wants to create
an int *and* has used an explicit cast.

Since you're asking for new checks to detect special cases involving time_t,
why not make it only warn about problem cases? Requiring
int(int64_t(time(nullptr))) here is not acceptable IMHO.

[Bug c++/96570] Warnings desired for time_t to int coversions

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96570

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |egallager at gcc dot gnu.org

--- Comment #6 from Eric Gallager <egallager at gcc dot gnu.org> ---
(In reply to M Welinder from comment #2)
> > Why? If somebody wants to be explicitly stupid, that's their prerogative.
> 
> I agree with the second sentence.
> 
> However, casts are not a clear indication that somebody wants to be
> explicitly stupid, at least not in C++.  If you were looking for such an
> indication, my int(int64_t(...)) suggestion is probably closer.
> 
> Casts occur also in (e.g.) overload resolution and entirely too often in
> template soup.  And in macros too, I guess.
> 

This reminds me of bug 69818