Bug 98660 - -Wold-style-cast should not warn on casts that look like (decltype(x))(x)
Summary: -Wold-style-cast should not warn on casts that look like (decltype(x))(x)
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2021-01-13 17:56 UTC by Gašper Ažman
Modified: 2021-01-17 15:15 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gašper Ažman 2021-01-13 17:56:15 UTC
Dear GCC wizards,

Recently, the use of std::forward has been idiomatically replaced by the following:

```
template <typename T>
void george(T&& x) {
   john((T&&)x); // means std::forward<T>(x)
}
```

Casting an `x` to `decltype(x)` is far shorter and faster to compile (which matters since it's done in unevaluated contexts A LOT). For an example, observe the usage in libunifex, the in-progress research implementation of the executors proposal:

https://github.com/facebookexperimental/libunifex/blob/master/include/unifex/tag_invoke.hpp

Unfortunately, the only real way to combine this fairly important exception to the general rules of "no c-style casts" is to disable -Wold-style-cast.

It would be a great benefit if I could leave that warning enabled, and sleep soundly in the knowledge I didn't mistype the forwarding expression if gcc checked for me that the type I'm casting to is, in fact, `decltype(x)`, and complain otherwise.

Please consider this refinement for a future release of GCC.

Thank you.
Comment 1 Ivan Sorokin 2021-01-14 00:42:44 UTC
I'm not a GCC developer, but I'm just curious.

Why the use of C-style cast is required here? Could you use static_cast instead? I mean instead of `(decltype(x))(x)` using `static_cast<decltype(x)>(x)`? Perhaps wrapping it in some macro in order to not duplicate `x` twice.
Comment 2 Gašper Ažman 2021-01-14 11:39:21 UTC
Ivan: indeed, you could use a static cast, or a macro - you're literally just changing the value category of the expression to its original one. The cast is safe.

The reason Niebler and friends (including me) are using the c-style cast is purely because it's short, concise, unambiguous, and fast to compile. `static_cast<decltype(x)>(x)` is a lot longer than `(T&&)x`. When you have 6 or 10 of these expressions (often with `...`s) in a function declaration (please see the `tag_invoke` definition linked), the noise starts to matter.
Comment 3 Eric Gallager 2021-01-16 21:09:38 UTC
Just don't use -Wold-style-cast then. Or disable it selectively with #pragma GCC diagnostic
Comment 4 Gašper Ažman 2021-01-16 21:54:04 UTC
@Eric Gallager: yes, the #pragma solution is what I currently use. It is entirely unsatisfactory, for the reasons described in my original request.

The long-term usefulness of warnings is directly proportional to the number of false positives they generate. If I wanted to enforce the warning in most of the code, and only disable it where appropriate, this is what that looks like, on a non-terrible example (borrowed from libunifex):

      #pragma GCC diagnostic push
      #pragma GCC diagnostic ignore "-Wold-style-cast"
      template <typename CPO, typename... Args>
      constexpr auto operator()(CPO cpo, Args&&... args) const
          noexcept(noexcept(tag_invoke((CPO &&) cpo, (Args &&) args...)))
          -> decltype(tag_invoke((CPO &&) cpo, (Args &&) args...)) {
        return tag_invoke((CPO &&) cpo, (Args &&) args...);
      }
      #endif

Now please, kindly tell me how I can know I didn't screw up a C-style cast in that mess? It turns out that in the example above, ALL the casts mean std::forward, but imagine it was a function where only some of the arguments are forwarded, and a co-worker mistakenly puts in a c-style cast that doesn't just change the value-category.

I'm asking for this warning to get more useful by still diagnosing c-style casts that change more than the value category, and not diagnosing c-style casts that are the "safest" kind - value category only, since that's what the idiom seems to be.
Comment 5 Gašper Ažman 2021-01-16 21:55:17 UTC
s/endif/pragma GCC diagnostic pop