[libcxx-commits] [PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

[Takuya Shimizu via Phabricator via libcxx-commits]

hazohelet added a comment.

In D155064#4651306 <https://reviews.llvm.org/D155064#4651306>, @hans wrote:

> We saw a new warning in Chromium after this, and I wasn't sure if that's intentional:
>
>   #include <limits>
>   
>   template <typename T> float foo(T input) {
>     if (sizeof(T) > 2) {
>       return 42;
>     } else {
>       constexpr float inverseMax = 1.0f / std::numeric_limits<T>::max();
>       return input * inverseMax;
>     }
>   }
>   
>   float f() {
>     return foo(1);
>   }
>   $ build/bin/clang -c /tmp/a.cc
>   /tmp/a.cc:7:41: warning: implicit conversion from 'int' to 'float' changes value from 2147483647 to 2147483648 [-Wimplicit-const-int-float-conversion]
>       7 |     constexpr float inverseMax = 1.0f / std::numeric_limits<T>::max();
>         |                                       ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/a.cc:13:10: note: in instantiation of function template specialization 'foo<int>' requested here
>      13 |   return foo(1);
>         |          ^
>   1 warning generated.
>
> Note that the branch with the conversion can be determined to not be taken based on the template argument.
>
> Using `if constexpr` suppresses the warning, but I'm not sure if that should really be necessary?
>
> (Our tracking bug is https://crbug.com/1487142)

It's unintended, but the cause is that I started using `Sema::Diag` instead of `Sema::DiagRuntimeBehavior`, in narrowing check for constexpr variable initializers, and thus lost the reachability analysis. It will be a simple fix, but would involve some adaptations to `Sema::DiagIfReachable`. I'll fix it after other concerns are resolved.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155064/new/

https://reviews.llvm.org/D155064