This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: C++ PATCH to fix an ICE on invalid with OVERLOADs (PR c++/84854)
On Thu, Mar 15, 2018 at 08:55:59AM -0400, Jason Merrill wrote:
> On Thu, Mar 15, 2018 at 7:49 AM, Marek Polacek <polacek@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 <polacek@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 <polacek@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 <polacek@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