| Message ID | Zg+rAIt6bpyXb7QZ@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Fix up maybe_warn_for_constant_evaluated calls [PR114580] | expand |
On Fri, Apr 05, 2024 at 09:40:48AM +0200, Jakub Jelinek wrote: > Hi! > > When looking at maybe_warn_for_constant_evaluated for the trivial > infinite loops patch, I've noticed that it can emit weird diagnostics > for if constexpr in templates, first warn that std::is_constant_evaluted() > always evaluates to false (because the function template is not constexpr) > and then during instantiation warn that std::is_constant_evaluted() > always evaluates to true (because it is used in if constexpr condition). > Now, only the latter is actually true, even when the if constexpr > is in a non-constexpr function, it will still always evaluate to true. > > So, the following patch fixes it to call maybe_warn_for_constant_evaluated > always with IF_STMT_CONSTEXPR_P (if_stmt) as the second argument rather than > true if it is if constexpr with non-dependent condition etc. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2024-04-05 Jakub Jelinek <jakub@redhat.com> > > PR c++/114580 > * semantics.cc (finish_if_stmt_cond): Call > maybe_warn_for_constant_evaluated with IF_STMT_CONSTEXPR_P (if_stmt) > as the second argument, rather than true/false depending on if > it is if constexpr with non-dependent constant expression with > bool type. > > * g++.dg/cpp2a/is-constant-evaluated15.C: New test. > > --- gcc/cp/semantics.cc.jj 2024-04-03 09:58:33.407772541 +0200 > +++ gcc/cp/semantics.cc 2024-04-04 12:11:36.203886572 +0200 > @@ -1126,6 +1126,9 @@ tree > finish_if_stmt_cond (tree orig_cond, tree if_stmt) > { > tree cond = maybe_convert_cond (orig_cond); > + maybe_warn_for_constant_evaluated (cond, > + /*constexpr_if=*/ > + IF_STMT_CONSTEXPR_P (if_stmt)); I don't think we need the comment anymore since it's clear what the argument does, and then the whole call can fit on a single line. But either way, the patch looks good, thanks. > if (IF_STMT_CONSTEXPR_P (if_stmt) > && !type_dependent_expression_p (cond) > && require_constant_expression (cond) > @@ -1134,16 +1137,11 @@ finish_if_stmt_cond (tree orig_cond, tre > converted to bool. */ > && TYPE_MAIN_VARIANT (TREE_TYPE (cond)) == boolean_type_node) > { > - maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/true); > cond = instantiate_non_dependent_expr (cond); > cond = cxx_constant_value (cond); > } > - else > - { > - maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/false); > - if (processing_template_decl) > - cond = orig_cond; > - } > + else if (processing_template_decl) > + cond = orig_cond; > finish_cond (&IF_COND (if_stmt), cond); > add_stmt (if_stmt); > THEN_CLAUSE (if_stmt) = push_stmt_list (); > --- gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated15.C.jj 2024-04-04 12:23:36.706962932 +0200 > +++ gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated15.C 2024-04-04 12:22:29.915882859 +0200 > @@ -0,0 +1,28 @@ > +// PR c++/114580 > +// { dg-do compile { target c++17 } } > +// { dg-options "-Wtautological-compare" } > + > +namespace std { > + constexpr inline bool > + is_constant_evaluated () noexcept > + { > +#if __cpp_if_consteval >= 202106L > + if consteval { return true; } else { return false; } > +#else > + return __builtin_is_constant_evaluated (); > +#endif > + } > +} > + > +template <typename T> > +void foo () > +{ > + if constexpr ((T) std::is_constant_evaluated ()) // { dg-warning "'std::is_constant_evaluated' always evaluates to true in 'if constexpr'" } > + ; // { dg-bogus "'std::is_constant_evaluated' always evaluates to false in a non-'constexpr' function" } > +} > + > +void > +bar () > +{ > + foo <bool> (); > +} > > Jakub > Marek
On 4/5/24 14:47, Marek Polacek wrote: > On Fri, Apr 05, 2024 at 09:40:48AM +0200, Jakub Jelinek wrote: >> Hi! >> >> When looking at maybe_warn_for_constant_evaluated for the trivial >> infinite loops patch, I've noticed that it can emit weird diagnostics >> for if constexpr in templates, first warn that std::is_constant_evaluted() >> always evaluates to false (because the function template is not constexpr) >> and then during instantiation warn that std::is_constant_evaluted() >> always evaluates to true (because it is used in if constexpr condition). >> Now, only the latter is actually true, even when the if constexpr >> is in a non-constexpr function, it will still always evaluate to true. >> >> So, the following patch fixes it to call maybe_warn_for_constant_evaluated >> always with IF_STMT_CONSTEXPR_P (if_stmt) as the second argument rather than >> true if it is if constexpr with non-dependent condition etc. >> >> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? >> >> 2024-04-05 Jakub Jelinek <jakub@redhat.com> >> >> PR c++/114580 >> * semantics.cc (finish_if_stmt_cond): Call >> maybe_warn_for_constant_evaluated with IF_STMT_CONSTEXPR_P (if_stmt) >> as the second argument, rather than true/false depending on if >> it is if constexpr with non-dependent constant expression with >> bool type. >> >> * g++.dg/cpp2a/is-constant-evaluated15.C: New test. >> >> --- gcc/cp/semantics.cc.jj 2024-04-03 09:58:33.407772541 +0200 >> +++ gcc/cp/semantics.cc 2024-04-04 12:11:36.203886572 +0200 >> @@ -1126,6 +1126,9 @@ tree >> finish_if_stmt_cond (tree orig_cond, tree if_stmt) >> { >> tree cond = maybe_convert_cond (orig_cond); >> + maybe_warn_for_constant_evaluated (cond, >> + /*constexpr_if=*/ >> + IF_STMT_CONSTEXPR_P (if_stmt)); > > I don't think we need the comment anymore since it's clear what the > argument does, and then the whole call can fit on a single line. > > But either way, the patch looks good, thanks. Agreed, OK with that change. Jason
--- gcc/cp/semantics.cc.jj 2024-04-03 09:58:33.407772541 +0200 +++ gcc/cp/semantics.cc 2024-04-04 12:11:36.203886572 +0200 @@ -1126,6 +1126,9 @@ tree finish_if_stmt_cond (tree orig_cond, tree if_stmt) { tree cond = maybe_convert_cond (orig_cond); + maybe_warn_for_constant_evaluated (cond, + /*constexpr_if=*/ + IF_STMT_CONSTEXPR_P (if_stmt)); if (IF_STMT_CONSTEXPR_P (if_stmt) && !type_dependent_expression_p (cond) && require_constant_expression (cond) @@ -1134,16 +1137,11 @@ finish_if_stmt_cond (tree orig_cond, tre converted to bool. */ && TYPE_MAIN_VARIANT (TREE_TYPE (cond)) == boolean_type_node) { - maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/true); cond = instantiate_non_dependent_expr (cond); cond = cxx_constant_value (cond); } - else - { - maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/false); - if (processing_template_decl) - cond = orig_cond; - } + else if (processing_template_decl) + cond = orig_cond; finish_cond (&IF_COND (if_stmt), cond); add_stmt (if_stmt); THEN_CLAUSE (if_stmt) = push_stmt_list (); --- gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated15.C.jj 2024-04-04 12:23:36.706962932 +0200 +++ gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated15.C 2024-04-04 12:22:29.915882859 +0200 @@ -0,0 +1,28 @@ +// PR c++/114580 +// { dg-do compile { target c++17 } } +// { dg-options "-Wtautological-compare" } + +namespace std { + constexpr inline bool + is_constant_evaluated () noexcept + { +#if __cpp_if_consteval >= 202106L + if consteval { return true; } else { return false; } +#else + return __builtin_is_constant_evaluated (); +#endif + } +} + +template <typename T> +void foo () +{ + if constexpr ((T) std::is_constant_evaluated ()) // { dg-warning "'std::is_constant_evaluated' always evaluates to true in 'if constexpr'" } + ; // { dg-bogus "'std::is_constant_evaluated' always evaluates to false in a non-'constexpr' function" } +} + +void +bar () +{ + foo <bool> (); +}