Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Unnecessary narrowing check in constexpr if (C++17) #38295

Closed Quuxplusone closed 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR39322
Status RESOLVED INVALID
Importance P enhancement
Reported by Kyrylo Bohdanenko (kyrylo.bohdanenko@gmail.com)
Reported on 2018-10-16 04:48:05 -0700
Last modified on 2018-10-25 02:05:43 -0700
Version trunk
Hardware All All
CC blitzrakete@gmail.com, kyrylo.bohdanenko@gmail.com, wielkiegie@gmail.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
Clang generates incorrect error about narrowing when compiling the following
code:

  int main() {
    constexpr int flags = 2;

    if constexpr (flags & 3) {
      return 0;
    }

    return 1;
  }

Errors:
prog.cc:5:17: error: constexpr if condition evaluates to 2, which cannot be
narrowed to type 'bool' [-Wc++11-narrowing]
  if constexpr (flags & 3) {
                ^

According to the standard the condition expression should be contextually
converted to bool (i.e. not considering the narrowing rules for "list
initialization" syntax):
* http://eel.is/c++draft/stmt.if#2
* http://eel.is/c++draft/conv#def:conversion,contextual_to_bool

The sandbox example is available on Wandbox:
https://wandbox.org/permlink/P4JWOgkWj8veamrf
Quuxplusone commented 6 years ago

May be related to #39322

Quuxplusone commented 6 years ago
(In reply to Kyrylo Bohdanenko from comment #1)
> May be related to #39322

I meant #30776
Quuxplusone commented 6 years ago

Nope, the condition is a "contextually converted constant expression of type bool;", which is something else. Namely, a narrowing conversion does not appear in the list of allowed conversions for such a conversion. http://eel.is/c++draft/expr.const#5

int -> bool is part of the narrowing conversions, as for clang sizeof(bool) < sizeof(int) and 2 doesn't fit inside a bool.

Quuxplusone commented 6 years ago

Yep, I see that I was wrong in my assumptions. Thanks for pointing out.

However, "contextually converted to bool" definition is slippery: is there any sense in defining this term if just "converted constant expression" would work the same.

Assuming that, we can try to interpret the definition a bit differently:

A contextually converted constant expression of type bool is an expression, contextually converted to bool, where the converted expression is a constant expression and the conversion sequence contains only the conversions above.

If we interpret "the converted expression" as "expression before contextually converting to bool" and "the conversion sequence" as "the conversions might have been performed in the converted expression (before contextually converting to bool)" this paragraph might receive new meaning. Which, in turn would leave us with only the "contextually converted to bool" requirement.

Anyway, lets hope the committee would consider changing the definition, as using integers inside a constexpr if has its use-cases. My own opinion is that enforcing such restriction on a language level (by hard errors) is not a good thing. Cases like above should instead generate a warning (with a separate -W* option, e.g. -Wconstexpr-bool-narrowing) so that later an exact policy about such situations could be specified by a "Style Guide" (e.g. "C++ Core Guidelines").

P.S. At the moment it looks like this should be reported as bug to GCC (because it does not consider such narrowing check).