On Thu, Mar 15, 2018 at 08:55:59AM -0400, Jason Merrill wrote: > On Thu, Mar 15, 2018 at 7:49 AM, Marek Polacek <pola...@redhat.com> wrote: > > On Wed, Mar 14, 2018 at 02:06:36PM -0400, Jason Merrill wrote: > >> On Wed, Mar 14, 2018 at 11:59 AM, Marek Polacek <pola...@redhat.com> wrote: > >> > cxx_constant_value doesn't understand template codes, and neither it > >> > understands OVERLOADs, so if we pass an OVERLOAD to it, we crash. Here > >> > instantiate_non_dependent_expr got an OVERLOAD, but since it calls > >> > is_nondependent_constant_expression which checks type_unknown_p, it left > >> > the > >> > expression as it was. We can't use is_nondependent_constant_expression > >> > in > >> > finish_if_stmt_cond because i_n_c_e checks is_constant_expression and > >> > that is > >> > not suitable here; we'd miss diagnostics. So I did the following; I > >> > think we > >> > should reject the testcase with an error. > >> > > >> > Bootstrapped/regtested on x86_64-linux, ok for trunk? > >> > > >> > 2018-03-14 Marek Polacek <pola...@redhat.com> > >> > > >> > PR c++/84854 > >> > * semantics.c (finish_if_stmt_cond): Give error if the condition > >> > is an overloaded function with no contextual type information. > >> > > >> > * g++.dg/cpp1z/constexpr-if15.C: New test. > >> > > >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c > >> > index fdf37bea770..a056e9445e9 100644 > >> > --- gcc/cp/semantics.c > >> > +++ gcc/cp/semantics.c > >> > @@ -735,8 +735,16 @@ finish_if_stmt_cond (tree cond, tree if_stmt) > >> > && require_constant_expression (cond) > >> > && !value_dependent_expression_p (cond)) > >> > { > >> > - cond = instantiate_non_dependent_expr (cond); > >> > - cond = cxx_constant_value (cond, NULL_TREE); > >> > + if (type_unknown_p (cond)) > >> > + { > >> > + cxx_incomplete_type_error (cond, TREE_TYPE (cond)); > >> > + cond = error_mark_node; > >> > >> I think I'd prefer to skip this block when type_unknown_p, and leave > >> error handling up to the code shared with regular if. > > > > Like this? > > Yes, thanks. > > > It was my first version, but I thought we wanted the error; > > Getting an error is an improvement, but it should apply to > non-constexpr if as well, so checking in maybe_convert_cond would be > better. Actually, if we deal with unknown type there, we shouldn't > need this patch at all. > > ...except I also notice that since maybe_convert_cond doesn't do any > conversion in a template, we're trying to extract the constant value > without first converting to bool, which breaks this testcase: > > struct A > { > constexpr operator bool () { return true; } > int i; > }; > > A a; > > template <class T> void f() > { > constexpr bool b = a; // works > if constexpr (a) { } // breaks > } > > int main() > { > f<int>(); > } > > A simple fix would be to replace your type_unknown_p check here with a > comparison to boolean_type_node, so we wait until instantiation time > to require a constant value.
Ok, that works. We should also make g++ accept the testcase with "static_assert(a)" instead of "if constexpr (a) { }" probably. I guess the cxx_constant_value call in finish_static_assert should happen earlier? > Better would be to actually do the conversion. Perhaps this could > share code (for converting and getting a constant value) with > finish_static_assert. But this I didn't manage to make to work. If I call perform_implicit_conversion_flags in maybe_convert_cond, I get error: cannot resolve overloaded function ‘foo’ based on conversion to type ‘bool’ so I'm not sure how the conversion would help. Anyway, here's at least the boolean_type_node version. Bootstrapped/regtested on x86_64-linux. 2018-03-21 Marek Polacek <pola...@redhat.com> PR c++/84854 * semantics.c (finish_if_stmt_cond): Check if the type of the condition is boolean. (finish_static_assert): Remove redundant variable. * g++.dg/cpp1z/constexpr-if15.C: New test. * g++.dg/cpp1z/constexpr-if16.C: New test. diff --git gcc/cp/semantics.c gcc/cp/semantics.c index fdf37bea770..39455c0168d 100644 --- gcc/cp/semantics.c +++ gcc/cp/semantics.c @@ -733,7 +733,10 @@ finish_if_stmt_cond (tree cond, tree if_stmt) if (IF_STMT_CONSTEXPR_P (if_stmt) && !type_dependent_expression_p (cond) && require_constant_expression (cond) - && !value_dependent_expression_p (cond)) + && !value_dependent_expression_p (cond) + /* Wait until instantiation time, since only then COND has been + converted to bool. */ + && TREE_TYPE (cond) == boolean_type_node) { cond = instantiate_non_dependent_expr (cond); cond = cxx_constant_value (cond, NULL_TREE); @@ -8619,8 +8622,6 @@ void finish_static_assert (tree condition, tree message, location_t location, bool member_p) { - tsubst_flags_t complain = tf_warning_or_error; - if (message == NULL_TREE || message == error_mark_node || condition == NULL_TREE @@ -8653,7 +8654,8 @@ finish_static_assert (tree condition, tree message, location_t location, /* Fold the expression and convert it to a boolean value. */ condition = perform_implicit_conversion_flags (boolean_type_node, condition, - complain, LOOKUP_NORMAL); + tf_warning_or_error, + LOOKUP_NORMAL); condition = fold_non_dependent_expr (condition); if (TREE_CODE (condition) == INTEGER_CST && !integer_zerop (condition)) diff --git gcc/testsuite/g++.dg/cpp1z/constexpr-if15.C gcc/testsuite/g++.dg/cpp1z/constexpr-if15.C index e69de29bb2d..9a9053c3305 100644 --- gcc/testsuite/g++.dg/cpp1z/constexpr-if15.C +++ gcc/testsuite/g++.dg/cpp1z/constexpr-if15.C @@ -0,0 +1,11 @@ +// PR c++/84854 +// { dg-options -std=c++17 } + +constexpr int foo () { return 1; } +constexpr int foo (int) { return 2; } + +template <typename> +void a() +{ + if constexpr(foo) { }; +} diff --git gcc/testsuite/g++.dg/cpp1z/constexpr-if16.C gcc/testsuite/g++.dg/cpp1z/constexpr-if16.C index e69de29bb2d..31a149702fd 100644 --- gcc/testsuite/g++.dg/cpp1z/constexpr-if16.C +++ gcc/testsuite/g++.dg/cpp1z/constexpr-if16.C @@ -0,0 +1,20 @@ +// { dg-options -std=c++17 } + +struct A +{ + constexpr operator bool () { return true; } + int i; +}; + +A a; + +template <class T> void f() +{ + constexpr bool b = a; + if constexpr (a) { } +} + +int main() +{ + f<int>(); +} Marek