c++: Distinguish unsatisfaction vs errors during satisfaction [PR97093]
authorPatrick Palka <ppalka@redhat.com>
Sat, 5 Dec 2020 18:47:22 +0000 (13:47 -0500)
committerPatrick Palka <ppalka@redhat.com>
Sat, 5 Dec 2020 18:47:22 +0000 (13:47 -0500)
During satisfaction, the flag info.noisy() controls three things:
whether to diagnose ill-formed satisfaction (such as the satisfaction
value of an atom being non-bool or non-constant); whether to diagnose
unsatisfaction; and whether to bypass the satisfaction cache.

The flag turns out to be too coarse however, because in some cases we
want to diagnose ill-formed satisfaction (and bypass the satisfaction
cache) but not diagnose unsatisfaction, for instance when replaying an
erroneous satisfaction result from constraint_satisfaction_value,
evaluate_concept_check and tsubst_nested_requirement.

And when noisily evaluating a disjunction, we want to first evaluate its
branches noisily (bypassing the satisfaction cache) but suppress
unsatisfaction diagnostics.  We currently work around this by instead
first evaluating each branch quietly, but that means the recursive calls
to satisfy_atom will use the satisfaction cache.

To fix this, this patch adds the info.diagnose_unsatisfaction_p() flag,
which refines the info.noisy() flag as part of a new sat_info class that
derives from subst_info.  During satisfaction, info.noisy() now controls
whether to diagnose ill-formed satisfaction, and
info.diagnose_unsatisfaction_p() controls whether to additionally
diagnose unsatisfaction.  This enables us to address the above two
issues straightforwardly.

Incidentally, the change to satisfy_disjunction suppresses the ICE in
the PR97093 testcase because we no longer insert atoms into the
satisfaction cache that have been incorrectly re-normalized in
diagnose_nested_requirement (after losing the necessary template
context).  But the underlying re-normalization issue remains, and will
be fixed in a subsequent patch.

gcc/cp/ChangeLog:

PR c++/97093
* constraint.cc (struct sat_info): Define.
(tsubst_nested_requirement): Pass a sat_info object to
satisfy_constraint.
(satisfy_constraint_r): Take a sat_info argument instead of
subst_info.
(satisfy_conjunction): Likewise.
(satisfy_disjunction): Likewise.  Instead of first evaluating
each branch quietly, evaluate each branch only with
unsatisfaction diagnostics disabled.  Exit early if evaluation
of a branch returns error_mark_node.
(satisfy_atom): Take a sat_info argument instead of subst_info.
Fix a comment.  Check diagnose_unsatisfaction_p() instead of
noisy() before replaying a substitution failure.
(satisfy_constraint): Take a sat_info argument instead of
subst_info.
(satisfy_associated_constraints): Likewise.
(satisfy_constraint_expression): Likewise.
(satisfy_declaration_constraints): Likewise.
(constraint_satisfaction_value): Likewise and adjust
accordingly.  Fix formatting.
(constraints_satisfied_p): Pass a sat_info object to
constraint_satisfaction_value.
(evaluate_concept_check): Pass a sat_info object to
satisfy_constraint_expression.
(diagnose_nested_requirement): Likewise.
(diagnose_constraints): Pass an appropriate sat_info object to
constraint_satisfaction_value.

gcc/testsuite/ChangeLog:

PR c++/97093
* g++.dg/concepts/pr94252.C: Verify we no longer issue a
spurious unsatisfaction note when diagnosing ill-formed
satisfaction.
* g++.dg/cpp2a/concepts-requires18.C: No longer expect a
spurious unsatisfaction diagnostic when evaluating the
nested-requirement subst<void&> of a requires-expression that
appears outside of a template.
* g++.dg/cpp2a/concepts-requires21.C: Verify we no longer issue
a spurious unsatisfaction note when evaluating a
nested-requirement of a requires-expression that appears outside
of a template.
* g++.dg/cpp2a/concepts-nonbool3.C: New test.
* g++.dg/cpp2a/concepts-pr97093.C: New test.

gcc/cp/constraint.cc
gcc/testsuite/g++.dg/concepts/pr94252.C
gcc/testsuite/g++.dg/cpp2a/concepts-nonbool3.C [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp2a/concepts-pr97093.C [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp2a/concepts-requires18.C
gcc/testsuite/g++.dg/cpp2a/concepts-requires21.C

index 00d2f2e..1117508 100644 (file)
@@ -98,7 +98,42 @@ struct subst_info
   tree in_decl;
 };
 
-static tree satisfy_constraint (tree, tree, subst_info);
+/* Provides additional context for satisfaction.
+
+   The flag noisy() controls whether to diagnose ill-formed satisfaction,
+   such as the satisfaction value of an atom being non-bool or non-constant.
+
+   The flag diagnose_unsatisfaction_p() controls whether to explain why
+   a constraint is not satisfied.
+
+   The entrypoints to satisfaction for which we set noisy+unsat are
+   diagnose_constraints and diagnose_nested_requirement.  The entrypoints for
+   which we set noisy-unsat are the replays inside constraint_satisfaction_value,
+   evaluate_concept_check and tsubst_nested_requirement.  In other entrypoints,
+   e.g. constraints_satisfied_p, we enter satisfaction quietly (both flags
+   cleared).  */
+
+struct sat_info : subst_info
+{
+  sat_info (tsubst_flags_t cmp, tree in, bool diag_unsat = false)
+    : subst_info (cmp, in), diagnose_unsatisfaction (diag_unsat)
+  {
+    if (diagnose_unsatisfaction_p ())
+      gcc_checking_assert (noisy ());
+  }
+
+  /* True if we should diagnose the cause of satisfaction failure.
+     Implies noisy().  */
+  bool
+  diagnose_unsatisfaction_p () const
+  {
+    return diagnose_unsatisfaction;
+  }
+
+  bool diagnose_unsatisfaction;
+};
+
+static tree satisfy_constraint (tree, tree, sat_info);
 
 /* True if T is known to be some type other than bool. Note that this
    is false for dependent types and errors.  */
@@ -2059,10 +2094,11 @@ tsubst_nested_requirement (tree t, tree args, subst_info info)
 {
   /* Ensure that we're in an evaluation context prior to satisfaction.  */
   tree norm = TREE_TYPE (t);
-  tree result = satisfy_constraint (norm, args, info);
+  tree result = satisfy_constraint (norm, args,
+                                   sat_info (info.complain, info.in_decl));
   if (result == error_mark_node && info.quiet ())
     {
-      subst_info noisy (tf_warning_or_error, info.in_decl);
+      sat_info noisy (tf_warning_or_error, info.in_decl);
       satisfy_constraint (norm, args, noisy);
     }
   if (result != boolean_true_node)
@@ -2499,12 +2535,12 @@ tsubst_constraint (tree t, tree args, tsubst_flags_t complain, tree in_decl)
   return expr;
 }
 
-static tree satisfy_constraint_r (tree, tree, subst_info info);
+static tree satisfy_constraint_r (tree, tree, sat_info info);
 
 /* Compute the satisfaction of a conjunction.  */
 
 static tree
-satisfy_conjunction (tree t, tree args, subst_info info)
+satisfy_conjunction (tree t, tree args, sat_info info)
 {
   tree lhs = satisfy_constraint_r (TREE_OPERAND (t, 0), args, info);
   if (lhs == error_mark_node || lhs == boolean_false_node)
@@ -2558,20 +2594,25 @@ collect_operands_of_disjunction (tree t, auto_vec<tree_pair> *operands)
 /* Compute the satisfaction of a disjunction.  */
 
 static tree
-satisfy_disjunction (tree t, tree args, subst_info info)
+satisfy_disjunction (tree t, tree args, sat_info info)
 {
-  /* Evaluate the operands quietly.  */
-  subst_info quiet (tf_none, NULL_TREE);
+  /* Evaluate each operand with unsatisfaction diagnostics disabled.  */
+  sat_info sub = info;
+  sub.diagnose_unsatisfaction = false;
 
-  /* Register the constraint for diagnostics, if needed.  */
-  diagnosing_failed_constraint failure (t, args, info.noisy ());
+  tree lhs = satisfy_constraint_r (TREE_OPERAND (t, 0), args, sub);
+  if (lhs == boolean_true_node || lhs == error_mark_node)
+    return lhs;
 
-  tree lhs = satisfy_constraint_r (TREE_OPERAND (t, 0), args, quiet);
-  if (lhs == boolean_true_node)
-    return boolean_true_node;
-  tree rhs = satisfy_constraint_r (TREE_OPERAND (t, 1), args, quiet);
-  if (rhs != boolean_true_node && info.noisy ())
+  tree rhs = satisfy_constraint_r (TREE_OPERAND (t, 1), args, sub);
+  if (rhs == boolean_true_node || rhs == error_mark_node)
+    return rhs;
+
+  /* Both branches evaluated to false.  Explain the satisfaction failure in
+     each branch.  */
+  if (info.diagnose_unsatisfaction_p ())
     {
+      diagnosing_failed_constraint failure (t, args, info.noisy ());
       cp_expr disj_expr = CONSTR_EXPR (t);
       inform (disj_expr.get_location (),
              "no operand of the disjunction is satisfied");
@@ -2592,7 +2633,8 @@ satisfy_disjunction (tree t, tree args, subst_info info)
            }
        }
     }
-  return rhs;
+
+  return boolean_false_node;
 }
 
 /* Ensures that T is a truth value and not (accidentally, as sometimes
@@ -2673,7 +2715,7 @@ static void diagnose_atomic_constraint (tree, tree, tree, subst_info);
 /* Compute the satisfaction of an atomic constraint.  */
 
 static tree
-satisfy_atom (tree t, tree args, subst_info info)
+satisfy_atom (tree t, tree args, sat_info info)
 {
   satisfaction_cache cache (t, args, info.complain);
   if (tree r = cache.get ())
@@ -2691,9 +2733,9 @@ satisfy_atom (tree t, tree args, subst_info info)
   tree map = tsubst_parameter_mapping (ATOMIC_CONSTR_MAP (t), args, quiet);
   if (map == error_mark_node)
     {
-      /* If instantiation of the parameter mapping fails, the program
-         is ill-formed.  */
-      if (info.noisy())
+      /* If instantiation of the parameter mapping fails, the constraint is
+        not satisfied.  Replay the substitution.  */
+      if (info.diagnose_unsatisfaction_p ())
        tsubst_parameter_mapping (ATOMIC_CONSTR_MAP (t), args, info);
       return cache.save (boolean_false_node);
     }
@@ -2720,7 +2762,7 @@ satisfy_atom (tree t, tree args, subst_info info)
     {
       /* If substitution results in an invalid type or expression, the constraint
         is not satisfied. Replay the substitution.  */
-      if (info.noisy ())
+      if (info.diagnose_unsatisfaction_p ())
        tsubst_expr (expr, args, info.complain, info.in_decl, false);
       return cache.save (inst_cache.save (boolean_false_node));
     }
@@ -2748,7 +2790,7 @@ satisfy_atom (tree t, tree args, subst_info info)
        result = error_mark_node;
     }
   result = satisfaction_value (result);
-  if (result == boolean_false_node && info.noisy ())
+  if (result == boolean_false_node && info.diagnose_unsatisfaction_p ())
     diagnose_atomic_constraint (t, map, result, info);
 
   return cache.save (inst_cache.save (result));
@@ -2766,7 +2808,7 @@ satisfy_atom (tree t, tree args, subst_info info)
    constraint only matters for subsumption.  */
 
 static tree
-satisfy_constraint_r (tree t, tree args, subst_info info)
+satisfy_constraint_r (tree t, tree args, sat_info info)
 {
   if (t == error_mark_node)
     return error_mark_node;
@@ -2787,7 +2829,7 @@ satisfy_constraint_r (tree t, tree args, subst_info info)
 /* Check that the normalized constraint T is satisfied for ARGS.  */
 
 static tree
-satisfy_constraint (tree t, tree args, subst_info info)
+satisfy_constraint (tree t, tree args, sat_info info)
 {
   auto_timevar time (TV_CONSTRAINT_SAT);
 
@@ -2805,7 +2847,7 @@ satisfy_constraint (tree t, tree args, subst_info info)
    value (either true, false, or error).  */
 
 static tree
-satisfy_associated_constraints (tree t, tree args, subst_info info)
+satisfy_associated_constraints (tree t, tree args, sat_info info)
 {
   /* If there are no constraints then this is trivially satisfied.  */
   if (!t)
@@ -2823,7 +2865,7 @@ satisfy_associated_constraints (tree t, tree args, subst_info info)
    satisfaction value. */
 
 static tree
-satisfy_constraint_expression (tree t, tree args, subst_info info)
+satisfy_constraint_expression (tree t, tree args, sat_info info)
 {
   if (t == error_mark_node)
     return error_mark_node;
@@ -2852,12 +2894,12 @@ satisfy_constraint_expression (tree t, tree args, subst_info info)
 tree
 satisfy_constraint_expression (tree expr)
 {
-  subst_info info (tf_none, NULL_TREE);
+  sat_info info (tf_none, NULL_TREE);
   return satisfy_constraint_expression (expr, NULL_TREE, info);
 }
 
 static tree
-satisfy_declaration_constraints (tree t, subst_info info)
+satisfy_declaration_constraints (tree t, sat_info info)
 {
   gcc_assert (DECL_P (t));
   const tree saved_t = t;
@@ -2917,7 +2959,7 @@ satisfy_declaration_constraints (tree t, subst_info info)
 }
 
 static tree
-satisfy_declaration_constraints (tree t, tree args, subst_info info)
+satisfy_declaration_constraints (tree t, tree args, sat_info info)
 {
   /* Update the declaration for diagnostics.  */
   info.in_decl = t;
@@ -2942,9 +2984,8 @@ satisfy_declaration_constraints (tree t, tree args, subst_info info)
 }
 
 static tree
-constraint_satisfaction_value (tree t, tsubst_flags_t complain)
+constraint_satisfaction_value (tree t, sat_info info)
 {
-  subst_info info (complain, NULL_TREE);
   tree r;
   if (DECL_P (t))
     r = satisfy_declaration_constraints (t, info);
@@ -2952,26 +2993,31 @@ constraint_satisfaction_value (tree t, tsubst_flags_t complain)
     r = satisfy_constraint_expression (t, NULL_TREE, info);
   if (r == error_mark_node && info.quiet ()
       && !(DECL_P (t) && TREE_NO_WARNING (t)))
-      {
-       constraint_satisfaction_value (t, tf_warning_or_error);
-       if (DECL_P (t))
-         /* Avoid giving these errors again.  */
-         TREE_NO_WARNING (t) = true;
-      }
+    {
+      /* Replay the error with re-normalized requirements.  */
+      sat_info noisy (tf_warning_or_error, info.in_decl);
+      constraint_satisfaction_value (t, noisy);
+      if (DECL_P (t))
+       /* Avoid giving these errors again.  */
+       TREE_NO_WARNING (t) = true;
+    }
   return r;
 }
 
 static tree
-constraint_satisfaction_value (tree t, tree args, tsubst_flags_t complain)
+constraint_satisfaction_value (tree t, tree args, sat_info info)
 {
-  subst_info info (complain, NULL_TREE);
   tree r;
   if (DECL_P (t))
     r = satisfy_declaration_constraints (t, args, info);
   else
     r = satisfy_constraint_expression (t, args, info);
   if (r == error_mark_node && info.quiet ())
-    constraint_satisfaction_value (t, args, tf_warning_or_error);
+    {
+      /* Replay the error with re-normalized requirements.  */
+      sat_info noisy (tf_warning_or_error, info.in_decl);
+      constraint_satisfaction_value (t, args, noisy);
+    }
   return r;
 }
 
@@ -2984,7 +3030,8 @@ constraints_satisfied_p (tree t)
   if (!flag_concepts)
     return true;
 
-  return constraint_satisfaction_value (t, tf_none) == boolean_true_node;
+  sat_info quiet (tf_none, NULL_TREE);
+  return constraint_satisfaction_value (t, quiet) == boolean_true_node;
 }
 
 /* True iff the result of satisfying T with ARGS is BOOLEAN_TRUE_NODE
@@ -2996,7 +3043,8 @@ constraints_satisfied_p (tree t, tree args)
   if (!flag_concepts)
     return true;
 
-  return constraint_satisfaction_value (t, args, tf_none) == boolean_true_node;
+  sat_info quiet (tf_none, NULL_TREE);
+  return constraint_satisfaction_value (t, args, quiet) == boolean_true_node;
 }
 
 /* Evaluate a concept check of the form C<ARGS>. This is only used for the
@@ -3011,14 +3059,14 @@ evaluate_concept_check (tree check, tsubst_flags_t complain)
   gcc_assert (concept_check_p (check));
 
   /* Check for satisfaction without diagnostics.  */
-  subst_info quiet (tf_none, NULL_TREE);
+  sat_info quiet (tf_none, NULL_TREE);
   tree result = satisfy_constraint_expression (check, NULL_TREE, quiet);
   if (result == error_mark_node && (complain & tf_error))
-  {
-    /* Replay the error with re-normalized requirements.  */
-    subst_info noisy (tf_warning_or_error, NULL_TREE);
-    satisfy_constraint_expression (check, NULL_TREE, noisy);
-  }
+    {
+      /* Replay the error with re-normalized requirements.  */
+      sat_info noisy (tf_warning_or_error, NULL_TREE);
+      satisfy_constraint_expression (check, NULL_TREE, noisy);
+    }
   return result;
 }
 
@@ -3496,7 +3544,7 @@ diagnose_nested_requirement (tree req, tree args)
   /* Quietly check for satisfaction first. We can elaborate details
      later if needed.  */
   tree norm = TREE_TYPE (req);
-  subst_info info (tf_none, NULL_TREE);
+  sat_info info (tf_none, NULL_TREE);
   tree result = satisfy_constraint (norm, args, info);
   if (result == boolean_true_node)
     return;
@@ -3507,7 +3555,7 @@ diagnose_nested_requirement (tree req, tree args)
     {
       /* Replay the substitution error.  */
       inform (loc, "nested requirement %qE is not satisfied, because", expr);
-      subst_info noisy (tf_warning_or_error, NULL_TREE);
+      sat_info noisy (tf_warning_or_error, NULL_TREE, /*diag_unsat=*/true);
       satisfy_constraint_expression (expr, args, noisy);
     }
   else
@@ -3651,11 +3699,12 @@ diagnose_constraints (location_t loc, tree t, tree args)
   if (concepts_diagnostics_max_depth == 0)
     return;
 
-  /* Replay satisfaction, but diagnose errors.  */
+  /* Replay satisfaction, but diagnose unsatisfaction.  */
+  sat_info noisy (tf_warning_or_error, NULL_TREE, /*diag_unsat=*/true);
   if (!args)
-    constraint_satisfaction_value (t, tf_warning_or_error);
+    constraint_satisfaction_value (t, noisy);
   else
-    constraint_satisfaction_value (t, args, tf_warning_or_error);
+    constraint_satisfaction_value (t, args, noisy);
 
   static bool suggested_p;
   if (concepts_diagnostics_max_depth_exceeded_p
index 56ce5f8..b045703 100644 (file)
@@ -16,6 +16,7 @@ static_assert(requires(S o, int i) {
 
 template<typename T>
   concept c = requires (T t) { requires (T)5; }; // { dg-error "has type .int." }
+// { dg-bogus "not satisfied" "" { target *-*-* } .-1 }
 
 int
 foo()
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-nonbool3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-nonbool3.C
new file mode 100644 (file)
index 0000000..2a2af54
--- /dev/null
@@ -0,0 +1,5 @@
+// { dg-do compile { target c++20 } }
+
+template <auto V> concept C = false || V || false; // { dg-error "has type 'int'" }
+template <auto V> int f() requires C<V>;
+int a = f<0>(); // { dg-error "no match" }
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr97093.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr97093.C
new file mode 100644 (file)
index 0000000..d662552
--- /dev/null
@@ -0,0 +1,32 @@
+// PR c++/97093
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-fconcepts-diagnostics-depth=3 --param=hash-table-verification-limit=10000" }
+
+template <typename T>
+concept C =  requires (T t)
+{
+  requires t.some_const < 2 || requires { t.some_fn (); };
+};
+
+template <unsigned, unsigned>
+struct c
+{};
+
+template <typename T>
+concept P = requires (T t, c <0, 1> v) { { t (v) }; }; // { dg-error "no match" }
+
+template <P auto, P auto ...>
+struct m
+{
+  constexpr auto operator () (C auto) const
+  {};
+};
+
+struct pc
+{
+  constexpr auto operator () (C auto) const
+  {};
+};
+
+constexpr auto cc = pc {};
+constexpr auto mmcc = m <cc> {}; // { dg-error "not satisfied" }
index a9b7720..9e45c58 100644 (file)
@@ -4,7 +4,7 @@ template<typename T>
 concept integer = __is_same_as(T, int);
 
 template<typename T>
-concept subst = requires (T x) { requires true; }; // { dg-error "parameter type .void." }
+concept subst = requires (T x) { requires true; };
 
 template<typename T>
 concept c1 = requires { requires integer<T> || subst<T&>; }; // { dg-message "in requirements" }
index bc38b89..8aead2f 100644 (file)
@@ -5,3 +5,4 @@ template<typename T, typename U>
 constexpr bool is_same_v = __is_same (T, U);
 
 static_assert(is_same_v<bool, decltype(requires { requires false; })>);
+// { dg-bogus "evaluated to 'false" "" { target *-*-* } .-1 }