[PATCH] c++: Diagnose unstable satisfaction results

Patrick Palka ppalka@redhat.com
Thu Dec 17 17:24:33 GMT 2020


On Thu, 17 Dec 2020, Patrick Palka wrote:

> On Fri, Dec 11, 2020 at 4:55 PM Patrick Palka <ppalka@redhat.com> wrote:
> >
> > On Fri, 11 Dec 2020, Jason Merrill wrote:
> >
> > > On 12/11/20 10:31 AM, Patrick Palka wrote:
> > > > On Thu, 10 Dec 2020, Patrick Palka wrote:
> > > >
> > > > > On Thu, 10 Dec 2020, Jason Merrill wrote:
> > > > >
> > > > > > On 12/10/20 11:21 AM, Patrick Palka wrote:
> > > > > > > This implements lightweight heuristical detection and diagnosing of
> > > > > > > satisfaction results that change at different points in the program,
> > > > > > > which renders the program as ill-formed NDR as of P2014.  We've
> > > > > > > recently
> > > > > > > started to more aggressively cache satisfaction results, and so the
> > > > > > > goal
> > > > > > > here is to make this caching behavior more transparent to users.
> > > > > > >
> > > > > > > A satisfaction result is flagged as "potentially unstable" (at the
> > > > > > > atom
> > > > > > > granularity) if during its computation, some type completion failure
> > > > > > > occurs.  This is detected by making complete_type_or_maybe_complain
> > > > > > > increment a counter upon failure and comparing the value of the
> > > > > > > counter
> > > > > > > before and after satisfaction.  (We don't instrument complete_type
> > > > > > > directly because it's used "opportunistically" in many spots where
> > > > > > > type
> > > > > > > completion failure doesn't necessary lead to substitution failure.)
> > > > > > >
> > > > > > > Flagged satisfaction results are always recomputed from scratch, even
> > > > > > > when performing satisfaction quietly.  We then compare the recomputed
> > > > > > > result with the cached result, and if they differ, proceed with
> > > > > > > diagnosing the instability.  (We may also unflag a result if it turned
> > > > > > > out to be independent of the previously detected type completion
> > > > > > > failure.)  When performing satisfaction noisily, we always check
> > > > > > > instability.
> > > > > > >
> > > > > > > Most of the implementation is confined to the satisfaction_cache
> > > > > > > class,
> > > > > > > which has been completely rewritten.
> > > > > > >
> > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on
> > > > > > > cmcstl2 and range-v3.  The static_assert failures in the view.join
> > > > > > > test
> > > > > > > from cmcstl2 are now elaborated on after this patch, and additionally
> > > > > > > the alg.equal_range test now fails for the same reason as the
> > > > > > > view.join
> > > > > > > test.
> > > > > > >
> > > > > > > gcc/cp/ChangeLog:
> > > > > > >
> > > > > > >       * constraint.cc (failed_type_completion_count): New.
> > > > > > >       (note_failed_type_completion_for_satisfaction): New.
> > > > > > >       (sat_entry::constr): Rename to ...
> > > > > > >       (sat_entry::atom): ... this.
> > > > > > >       (sat_entry::location): New member.
> > > > > > >       (sat_entry::maybe_unstable): New member.
> > > > > > >       (sat_entry::diagnose_instability): New member.
> > > > > > >       (struct sat_hasher): Adjust after the above renaming.
> > > > > > >       (get_satisfaction, save_satisfaction): Remove.
> > > > > > >       (satisfaction_cache): Rewrite completely.
> > > > > > >       (satisfy_atom): When instantiation of the parameter mapping
> > > > > > >       fails, set diagnose_instability.  Propagate location from
> > > > > > >       inst_cache.entry to cache.entry if the secondary lookup
> > > > > > >       succeeded.
> > > > > > >       (satisfy_declaration_constraints): When
> > > > > > >       failed_type_completion_count differs before and after
> > > > > > >       satisfaction, then don't cache the satisfaction result.
> > > > > > >       * cp-tree.h (note_failed_type_completion_for_satisfaction):
> > > > > > >       Declare.
> > > > > > >       * pt.c (tsubst) <case TYPENAME_TYPE>: Use
> > > > > > >       complete_type_or_maybe_complain instead of open-coding it.
> > > > > > >       * typeck.c (complete_type_or_maybe_complain): Call
> > > > > > >       note_failed_type_completion_for_satisfaction when type
> > > > > > >       completion fails.
> > > > > > >
> > > > > > > gcc/testsuite/ChangeLog:
> > > > > > >
> > > > > > >       * g++.dg/cpp2a/concepts-complete1.C: New test.
> > > > > > >       * g++.dg/cpp2a/concepts-complete2.C: New test.
> > > > > > >       * g++.dg/cpp2a/concepts-complete3.C: New test.
> > > > > > > ---
> > > > > > >    gcc/cp/constraint.cc                          | 283
> > > > > > > ++++++++++++++----
> > > > > > >    gcc/cp/cp-tree.h                              |   2 +
> > > > > > >    gcc/cp/pt.c                                   |   9 +-
> > > > > > >    gcc/cp/typeck.c                               |   1 +
> > > > > > >    .../g++.dg/cpp2a/concepts-complete1.C         |  18 ++
> > > > > > >    .../g++.dg/cpp2a/concepts-complete2.C         |  23 ++
> > > > > > >    .../g++.dg/cpp2a/concepts-complete3.C         |  16 +
> > > > > > >    7 files changed, 282 insertions(+), 70 deletions(-)
> > > > > > >    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> > > > > > >    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-complete2.C
> > > > > > >    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-complete3.C
> > > > > > >
> > > > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > > > > > > index 73c038e3afe..ee702b34d01 100644
> > > > > > > --- a/gcc/cp/constraint.cc
> > > > > > > +++ b/gcc/cp/constraint.cc
> > > > > > > @@ -2374,35 +2374,82 @@ tsubst_parameter_mapping (tree map, tree args,
> > > > > > > tsubst_flags_t complain, tree in_
> > > > > > >                            Constraint satisfaction
> > > > > > >    ---------------------------------------------------------------------------*/
> > > > > > >    -/* Hash functions for satisfaction entries.  */
> > > > > > > +/* A counter incremented by
> > > > > > > note_failed_type_completion_for_satisfaction().
> > > > > > > +   It's used by the satisfaction caches in order to flag "potentially
> > > > > > > unstable"
> > > > > > > +   satisfaction results.  */
> > > > > > > +
> > > > > > > +static unsigned failed_type_completion_count;
> > > > > > > +
> > > > > > > +/* Called whenever a type completion failure occurs that definitely
> > > > > > > affects
> > > > > > > +   the semantics of the program, by e.g. inducing substitution
> > > > > > > failure.  */
> > > > > > > +
> > > > > > > +void
> > > > > > > +note_failed_type_completion_for_satisfaction (tree type)
> > > > > > > +{
> > > > > > > +  gcc_checking_assert (!COMPLETE_TYPE_P (type));
> > > > > > > +  if (CLASS_TYPE_P (type)
> > > > > > > +      && CLASSTYPE_TEMPLATE_INSTANTIATION (type))
> > > > > > > +    /* After instantiation, an incomplete class template
> > > > > > > specialization
> > > > > > > +       will always be incomplete, so we don't increment the counter
> > > > > > > in this
> > > > > > > +       case.  */;
> > > > > >
> > > > > > Hmm?  If a class template is declared early in the TU and defined later,
> > > > > > a
> > > > > > later attempt will succeed in instantiating it.
> > > > >
> > > > > But shouldn't the result of the first instantiation, the one that yields
> > > > > an incomplete class type, prevail for the entire TU?  At least, that's
> > > > > how I understand [temp.inst]/1 and [temp.point]/7.
> > > >
> > > > Oops, I meant to refer to [temp.inst]/2 here instead.  In particular,
> > > > the sentences
> > > >
> > > >   from [temp.inst]/2:
> > > >     If a class template has been declared, but not defined, at the point
> > > >     of instantiation, the instantiation yields an incomplete class type.
> > > >
> > > >   from [temp.point]/7:
> > > >     A specialization for a class template has at most one point of
> > > >     instantiation within a translation unit.
> > > >
> > > > together seem to imply it's impossible to complete a class template
> > > > specialization after an earlier instantiation yielded an incomplete
> > > > type, for then there would be two different points of instantiation
> > > > for the same specialization.
> > >
> > > Interesting.  No implementation I know of shares this interpretation; existing
> > > practice is that subsequent potential points of instantiation will try again.
> > > For instance, this is accepted by g++, clang, EDG, and VC++:
> > >
> > > template <class T> struct A;
> > > struct B { };
> > >
> > > int f(...);
> > > int f(B*);
> > >
> > > A<int> *p;
> > >
> > > int i = f(p); // can't instantiate A<int>, calls f(...)
> > > template <class T> struct A: B { };
> > > int j = f(p); // instantiates A<int>, calls f(B*)
> > >
> > > So the compilers don't treat the first call as an actual point of
> > > instantiation for A<int>.
> >
> > I see.  I wonder if the standard could use some clarification here then,
> > perhaps [temp.point] could say "... has at most one point of
> > instantiation within a translate unit _that yields a complete type_"
> > or something like that.
> >
> > > I'm curious how much code would break if we
> > > enforced your interpretation.
> >
> > I ran a bootstrap and regtest with the following that prevents us from
> > attempting to instantiate an incomplete class template
> > specialization more than once:
> >
> > -- >8 --
> >
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index e5f18d23ea0..d1fe621e311 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -11704,6 +11704,8 @@ perform_instantiation_time_access_checks (tree tmpl, tree targs)
> >        }
> >  }
> >
> > +static GTY(()) hash_set<tree, false, tree_type_hash> *incomplete_at_poi;
> > +
> >  static tree
> >  instantiate_class_template_1 (tree type)
> >  {
> > @@ -11717,8 +11719,12 @@ instantiate_class_template_1 (tree type)
> >    if (type == error_mark_node)
> >      return error_mark_node;
> >
> > +  if (!incomplete_at_poi)
> > +    incomplete_at_poi = hash_set<tree, false, tree_type_hash>::create_ggc (31);
> > +
> >    if (COMPLETE_OR_OPEN_TYPE_P (type)
> > -      || uses_template_parms (type))
> > +      || uses_template_parms (type)
> > +      || incomplete_at_poi->contains (type))
> >      return type;
> >
> >    /* Figure out which template is being instantiated.  */
> > @@ -11764,6 +11770,7 @@ instantiate_class_template_1 (tree type)
> >      {
> >        /* We can try again later.  */
> >        TYPE_BEING_DEFINED (type) = 0;
> > +      incomplete_at_poi->add (type);
> >        return type;
> >      }
> >
> > -- >8 --
> >
> > and the only FAILs were excess incompleteness errors in the invalid
> > testcases template/nontype[45].C.
> >
> > So far I noticed the patch breaks clang, because a couple of header
> > files use an incomplete class template specialization inside a function
> > declaration before later defining the class and using the function, e.g.
> >
> >   template <class> struct S;
> >   void foo (S<int>);
> >   ...
> >   // define S
> >   ...
> >   // call foo()
> >
> > I reckon this pattern is not uncommon in the wild..
> >
> > So I guess this assumption in note_failed_type_completion_for_satisfaction
> > is unsound in practice, sadly.  It was added to avoid a pretty big
> > compile time/memory use regression for the view.take cmcstl2 testcase,
> > due to a frequently used atom that checks for completeness of
> > std::tuple_size<T>, and is frequently applied to a non-tuple T, so we
> > end up recomputing, in vain, the corresponding cached satisfaction
> > result.
> >
> > An earlier prototype of this patch kept precise track of the types for
> > which completion failed during satisfaction (as a pair of integer
> > offsets into a global failed_type_completions vector).  And the
> > prototype recomputed the cached satisfaction result only when one of
> > these incomplete types became complete.
> >
> > Later, for the sake of simplicity, I replaced this with the
> > failed_type_completion_count counter and the coarse-grained
> > sat_entry::maybe_unstable_p flag.  But I will look into re-adding this
> > more fine-grained tracking since I think it should resolve the
> > performance regression.
> 
> Thanks a lot for the reviews.  Does this first patch in the series
> look OK as well?

To be more precise, the patch in question is ...

> 
> >
> > >
> > > > > FWIW, this "optimization" was added in order to avoid a significant
> > > > > performance regression in cmcstl's view.take testcase, which ends up
> > > > > frequently checking an unsatisfiable atom that wants completeness of
> > > > > std::tuple_size<T>.
> > >
> > > > > > > +  else
> > > > > > > +    ++failed_type_completion_count;
> > > > > > > +}
> > > > > > > +
> > > > > > > +/* Hash functions and data types for satisfaction cache entries.  */
> > > > > > >      struct GTY((for_user)) sat_entry
> > > > > > >    {
> > > > > > > -  tree constr;
> > > > > > > +  /* The relevant ATOMIC_CONSTR.  */
> > > > > > > +  tree atom;
> > > > > > > +
> > > > > > > +  /* The relevant template arguments.  */
> > > > > > >      tree args;
> > > > > > > +
> > > > > > > +  /* The result of satisfaction of ATOM+ARGS.
> > > > > > > +     This is either boolean_true_node, boolean_false_node or
> > > > > > > error_mark_node,
> > > > > > > +     where error_mark_node indicates ill-formed satisfaction.
> > > > > > > +     It's set to NULL_TREE while computing satisfaction of ATOM+ARGS
> > > > > > > for
> > > > > > > +     the first time.  */
> > > > > > >      tree result;
> > > > > > > +
> > > > > > > +  /* The value of input_location when satisfaction of ATOM+ARGS was
> > > > > > > first
> > > > > > > +     performed.  */
> > > > > > > +  location_t location;
> > > > > > > +
> > > > > > > +  /* True if this satisfaction result is flagged as "potentially
> > > > > > > unstable",
> > > > > > > +     i.e. the result might change at different points in the program
> > > > > > > if
> > > > > > > +     recomputed from scratch (which would be UB).  This flag is used
> > > > > > > to
> > > > > > > +     heuristically diagnose such UB when it occurs, by recomputing
> > > > > > > this
> > > > > > > +     satisfaction from scratch even when evaluating quietly.  */
> > > > > > > +  bool maybe_unstable;
> > > > > > > +
> > > > > > > +  /* True if we want to diagnose the above instability when it's
> > > > > > > detected.
> > > > > > > +     We don't always want to do so, in order to avoid emitting
> > > > > > > duplicate
> > > > > > > +     diagnostics in some cases.  */
> > > > > > > +  bool diagnose_instability;
> > > > > > >    };
> > > > > > >      struct sat_hasher : ggc_ptr_hash<sat_entry>
> > > > > > >    {
> > > > > > >      static hashval_t hash (sat_entry *e)
> > > > > > >      {
> > > > > > > -    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e->constr))
> > > > > > > +    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e->atom))
> > > > > > >          {
> > > > > > >       /* Atoms with instantiated mappings are built during
> > > > > > > satisfaction.
> > > > > > >          They live only inside the sat_cache, and we build one to
> > > > > > > query
> > > > > > >          the cache with each time we instantiate a mapping.  */
> > > > > > >       gcc_assert (!e->args);
> > > > > > > -     return hash_atomic_constraint (e->constr);
> > > > > > > +     return hash_atomic_constraint (e->atom);
> > > > > > >          }
> > > > > > >          /* Atoms with uninstantiated mappings are built during
> > > > > > > normalization.
> > > > > > >           Since normalize_atom caches the atoms it returns, we can
> > > > > > > assume
> > > > > > >           pointer-based identity for fast hashing and comparison.
> > > > > > > Even if
> > > > > > > this
> > > > > > >           assumption is violated, that's okay, we'll just get a cache
> > > > > > > miss.
> > > > > > > */
> > > > > > > -    hashval_t value = htab_hash_pointer (e->constr);
> > > > > > > +    hashval_t value = htab_hash_pointer (e->atom);
> > > > > > >    -    if (tree map = ATOMIC_CONSTR_MAP (e->constr))
> > > > > > > +    if (tree map = ATOMIC_CONSTR_MAP (e->atom))
> > > > > > >          /* Only the parameters that are used in the targets of the
> > > > > > > mapping
> > > > > > >        affect the satisfaction value of the atom.  So we consider
> > > > > > > only
> > > > > > >        the arguments for these parameters, and ignore the rest.  */
> > > > > > > @@ -2425,21 +2472,21 @@ struct sat_hasher : ggc_ptr_hash<sat_entry>
> > > > > > >        static bool equal (sat_entry *e1, sat_entry *e2)
> > > > > > >      {
> > > > > > > -    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->constr)
> > > > > > > -     != ATOMIC_CONSTR_MAP_INSTANTIATED_P (e2->constr))
> > > > > > > +    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->atom)
> > > > > > > +     != ATOMIC_CONSTR_MAP_INSTANTIATED_P (e2->atom))
> > > > > > >          return false;
> > > > > > >          /* See sat_hasher::hash.  */
> > > > > > > -    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->constr))
> > > > > > > +    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->atom))
> > > > > > >          {
> > > > > > >       gcc_assert (!e1->args && !e2->args);
> > > > > > > -     return atomic_constraints_identical_p (e1->constr,
> > > > > > > e2->constr);
> > > > > > > +     return atomic_constraints_identical_p (e1->atom, e2->atom);
> > > > > > >          }
> > > > > > >    -    if (e1->constr != e2->constr)
> > > > > > > +    if (e1->atom != e2->atom)
> > > > > > >          return false;
> > > > > > >    -    if (tree map = ATOMIC_CONSTR_MAP (e1->constr))
> > > > > > > +    if (tree map = ATOMIC_CONSTR_MAP (e1->atom))
> > > > > > >          for (tree target_parms = TREE_TYPE (map);
> > > > > > >          target_parms;
> > > > > > >          target_parms = TREE_CHAIN (target_parms))
> > > > > > > @@ -2467,59 +2514,146 @@ static GTY((deletable))
> > > > > > > hash_table<sat_hasher>
> > > > > > > *sat_cache;
> > > > > > >    /* Cache the result of constraint_satisfaction_value.  */
> > > > > > >    static GTY((deletable)) hash_map<tree, tree> *decl_satisfied_cache;
> > > > > > >    -static tree
> > > > > > > -get_satisfaction (tree constr, tree args)
> > > > > > > -{
> > > > > > > -  if (!sat_cache)
> > > > > > > -    return NULL_TREE;
> > > > > > > -  sat_entry elt = { constr, args, NULL_TREE };
> > > > > > > -  sat_entry* found = sat_cache->find (&elt);
> > > > > > > -  if (found)
> > > > > > > -    return found->result;
> > > > > > > -  else
> > > > > > > -    return NULL_TREE;
> > > > > > > -}
> > > > > > > -
> > > > > > > -static void
> > > > > > > -save_satisfaction (tree constr, tree args, tree result)
> > > > > > > -{
> > > > > > > -  if (!sat_cache)
> > > > > > > -    sat_cache = hash_table<sat_hasher>::create_ggc (31);
> > > > > > > -  sat_entry elt = {constr, args, result};
> > > > > > > -  sat_entry** slot = sat_cache->find_slot (&elt, INSERT);
> > > > > > > -  sat_entry* entry = ggc_alloc<sat_entry> ();
> > > > > > > -  *entry = elt;
> > > > > > > -  *slot = entry;
> > > > > > > -}
> > > > > > > -
> > > > > > > -/* A tool to help manage satisfaction caching in
> > > > > > > satisfy_constraint_r.
> > > > > > > -   Note the cache is only used when not diagnosing errors.  */
> > > > > > > +/* A tool used by satisfy_atom to help manage satisfaction caching
> > > > > > > and to
> > > > > > > +   diagnose "unstable" satisfaction values.  We insert into the cache
> > > > > > > only
> > > > > > > +   when performing satisfaction quietly.  */
> > > > > > >      struct satisfaction_cache
> > > > > > >    {
> > > > > > > -  satisfaction_cache (tree constr, tree args, tsubst_flags_t
> > > > > > > complain)
> > > > > > > -    : constr(constr), args(args), complain(complain)
> > > > > > > -  { }
> > > > > > > +  satisfaction_cache (tree, tree, sat_info);
> > > > > > > +  tree get ();
> > > > > > > +  tree save (tree);
> > > > > > >    -  tree get ()
> > > > > > > -  {
> > > > > > > -    if (complain == tf_none)
> > > > > > > -      return get_satisfaction (constr, args);
> > > > > > > -    return NULL_TREE;
> > > > > > > -  }
> > > > > > > -
> > > > > > > -  tree save (tree result)
> > > > > > > -  {
> > > > > > > -    if (complain == tf_none)
> > > > > > > -      save_satisfaction (constr, args, result);
> > > > > > > -    return result;
> > > > > > > -  }
> > > > > > > -
> > > > > > > -  tree constr;
> > > > > > > -  tree args;
> > > > > > > -  tsubst_flags_t complain;
> > > > > > > +  sat_entry *entry;
> > > > > > > +  sat_info info;
> > > > > > > +  unsigned ftc_count;
> > > > > > >    };
> > > > > > >    +/* Constructor for the satisfaction_cache class.  We're performing
> > > > > > > satisfaction
> > > > > > > +   of ATOM+ARGS according to INFO.  */
> > > > > > > +
> > > > > > > +satisfaction_cache
> > > > > > > +::satisfaction_cache (tree atom, tree args, sat_info info)
> > > > > > > +  : entry(nullptr), info(info),
> > > > > > > ftc_count(failed_type_completion_count)
> > > > > > > +{
> > > > > > > +  if (!sat_cache)
> > > > > > > +    sat_cache = hash_table<sat_hasher>::create_ggc (31);
> > > > > > > +
> > > > > > > +  /* When noisy, we query the satisfaction cache in order to diagnose
> > > > > > > +     "unstable" satisfaction values.  */
> > > > > > > +  if (info.noisy ())
> > > > > > > +    {
> > > > > > > +      /* When noisy, constraints have been re-normalized, and that
> > > > > > > breaks
> > > > > > > the
> > > > > > > +      pointer-based identity assumption of sat_cache (for atoms
> > > > > > > with
> > > > > > > +      uninstantiated mappings).  So undo this re-normalization by
> > > > > > > looking
> > > > > > > in
> > > > > > > +      the atom_cache for the corresponding atom that was used
> > > > > > > during quiet
> > > > > > > +      satisfaction.  */
> > > > > > > +      if (!ATOMIC_CONSTR_MAP_INSTANTIATED_P (atom))
> > > > > > > +     {
> > > > > > > +       if (tree found = atom_cache->find (atom))
> > > > > > > +         atom = found;
> > > > > > > +       else
> > > > > > > +         /* The lookup should always succeed, but if it fails then
> > > > > > > let's
> > > > > > > +            just leave 'entry' empty, effectively disabling the
> > > > > > > cache.  */
> > > > > > > +         return;
> > > > > > > +     }
> > > > > > > +    }
> > > > > > > +
> > > > > > > +  /* Look up or create the corresponding satisfaction entry.  */
> > > > > > > +  sat_entry elt;
> > > > > > > +  elt.atom = atom;
> > > > > > > +  elt.args = args;
> > > > > > > +  sat_entry **slot = sat_cache->find_slot (&elt, INSERT);
> > > > > > > +  if (*slot)
> > > > > > > +    entry = *slot;
> > > > > > > +  else if (info.quiet ())
> > > > > > > +    {
> > > > > > > +      entry = ggc_alloc<sat_entry> ();
> > > > > > > +      entry->atom = atom;
> > > > > > > +      entry->args = args;
> > > > > > > +      entry->result = NULL_TREE;
> > > > > > > +      entry->location = input_location;
> > > > > > > +      entry->maybe_unstable = false;
> > > > > > > +      entry->diagnose_instability = false;
> > > > > > > +      if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (atom))
> > > > > > > +     /* We always want to diagnose instability of an atom with an
> > > > > > > +        instantiated parameter mapping.  For atoms with an
> > > > > > > uninstiantiated
> > > > > >
> > > > > > Extra 'i' in "uninstantiated"
> > > > >
> > > > > Fixed, as well as applied David's suggestion to use
> > > > > auto_diagnostic_group.  Here's an updated patch which improves some
> > > > > comments too, no functional changes:
> > > > >

... this one below.  (I'll commit the already-approved follow-up patch
https://gcc.gnu.org/pipermail/gcc-patches/2020-December/562131.html
which replaces the aforementioned invalid assumption in
note_failed_type_completion_for_satisfaction separately.)

> > > > > -- >8 --
> > > > >
> > > > > Subject: [PATCH] c++: Diagnose unstable satisfaction
> > > > >
> > > > > This implements lightweight heuristical detection and diagnosing of
> > > > > satisfaction whose result changes at different points in the program,
> > > > > which renders the program ill-formed NDR as of P2104.  We've recently
> > > > > started to more aggressively cache satisfaction results, and so the goal
> > > > > with this patch is to make this caching behavior more transparent to
> > > > > the user.
> > > > >
> > > > > A satisfaction result is flagged as "potentially unstable" (at the atom
> > > > > granularity) if during its computation, some type completion failure
> > > > > occurs.  This is detected by making complete_type_or_maybe_complain
> > > > > increment a counter upon failure and comparing the value of the counter
> > > > > before and after satisfaction.  (We don't instrument complete_type
> > > > > directly because it's used "opportunistically" in many spots where type
> > > > > completion failure doesn't necessary lead to substitution failure.)
> > > > >
> > > > > Such flagged satisfaction results are always recomputed from scratch,
> > > > > even when performing satisfaction quietly.  When saving a satisfaction
> > > > > result, we now compare the computed result with the cached result, and
> > > > > if they differ, proceed with diagnosing the instability.
> > > > >
> > > > > Most of the implementation is confined to the satisfaction_cache class,
> > > > > which has been completely rewritten.
> > > > >
> > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on
> > > > > cmcstl2 and range-v3.  The static_assert failures in the view.join test
> > > > > from cmcstl2 are now elaborated on after this patch, and additionally
> > > > > the alg.equal_range test now fails for the same reason as the view.join
> > > > > test.
> > > > >
> > > > > gcc/cp/ChangeLog:
> > > > >
> > > > >   * constraint.cc (failed_type_completion_count): New.
> > > > >   (note_failed_type_completion_for_satisfaction): New.
> > > > >   (sat_entry::constr): Rename to ...
> > > > >   (sat_entry::atom): ... this.
> > > > >   (sat_entry::location): New member.
> > > > >   (sat_entry::maybe_unstable): New member.
> > > > >   (sat_entry::diagnose_instability): New member.
> > > > >   (struct sat_hasher): Adjust after the above renaming.
> > > > >   (get_satisfaction, save_satisfaction): Remove.
> > > > >   (satisfaction_cache): Rewrite completely.
> > > > >   (satisfy_atom): When instantiation of the parameter mapping
> > > > >   fails, set diagnose_instability.  Propagate location from
> > > > >   inst_cache.entry to cache.entry if the secondary lookup
> > > > >   succeeded.
> > > > >   (satisfy_declaration_constraints): When
> > > > >   failed_type_completion_count differs before and after
> > > > >   satisfaction, then don't cache the satisfaction result.
> > > > >   * cp-tree.h (note_failed_type_completion_for_satisfaction):
> > > > >   Declare.
> > > > >   * pt.c (tsubst) <case TYPENAME_TYPE>: Use
> > > > >   complete_type_or_maybe_complain instead of open-coding it.
> > > > >   * typeck.c (complete_type_or_maybe_complain): Call
> > > > >   note_failed_type_completion_for_satisfaction when type
> > > > >   completion fails.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > >   * g++.dg/cpp2a/concepts-complete1.C: New test.
> > > > >   * g++.dg/cpp2a/concepts-complete2.C: New test.
> > > > >   * g++.dg/cpp2a/concepts-complete3.C: New test.
> > > > > ---
> > > > >   gcc/cp/constraint.cc                          | 283 ++++++++++++++----
> > > > >   gcc/cp/cp-tree.h                              |   2 +
> > > > >   gcc/cp/pt.c                                   |   9 +-
> > > > >   gcc/cp/typeck.c                               |   1 +
> > > > >   .../g++.dg/cpp2a/concepts-complete1.C         |  18 ++
> > > > >   .../g++.dg/cpp2a/concepts-complete2.C         |  23 ++
> > > > >   .../g++.dg/cpp2a/concepts-complete3.C         |  16 +
> > > > >   7 files changed, 282 insertions(+), 70 deletions(-)
> > > > >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> > > > >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-complete2.C
> > > > >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-complete3.C
> > > > >
> > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > > > > index 73c038e3afe..b98befaf71b 100644
> > > > > --- a/gcc/cp/constraint.cc
> > > > > +++ b/gcc/cp/constraint.cc
> > > > > @@ -2374,35 +2374,82 @@ tsubst_parameter_mapping (tree map, tree args,
> > > > > tsubst_flags_t complain, tree in_
> > > > >                           Constraint satisfaction
> > > > >   ---------------------------------------------------------------------------*/
> > > > >   -/* Hash functions for satisfaction entries.  */
> > > > > +/* A counter incremented by
> > > > > note_failed_type_completion_for_satisfaction().
> > > > > +   It's used by the satisfaction caches in order to flag "potentially
> > > > > unstable"
> > > > > +   satisfaction results.  */
> > > > > +
> > > > > +static unsigned failed_type_completion_count;
> > > > > +
> > > > > +/* Called whenever a type completion failure occurs that definitely
> > > > > affects
> > > > > +   the semantics of the program, by e.g. inducing substitution failure.
> > > > > */
> > > > > +
> > > > > +void
> > > > > +note_failed_type_completion_for_satisfaction (tree type)
> > > > > +{
> > > > > +  gcc_checking_assert (!COMPLETE_TYPE_P (type));
> > > > > +  if (CLASS_TYPE_P (type)
> > > > > +      && CLASSTYPE_TEMPLATE_INSTANTIATION (type))
> > > > > +    /* After instantiation, a class template specialization that's
> > > > > +       incomplete will remain incomplete, so for our purposes we can
> > > > > +       ignore this completion failure event.  */;
> > > > > +  else
> > > > > +    ++failed_type_completion_count;
> > > > > +}
> > > > > +
> > > > > +/* Hash functions and data types for satisfaction cache entries.  */
> > > > >     struct GTY((for_user)) sat_entry
> > > > >   {
> > > > > -  tree constr;
> > > > > +  /* The relevant ATOMIC_CONSTR.  */
> > > > > +  tree atom;
> > > > > +
> > > > > +  /* The relevant template arguments.  */
> > > > >     tree args;
> > > > > +
> > > > > +  /* The result of satisfaction of ATOM+ARGS.
> > > > > +     This is either boolean_true_node, boolean_false_node or
> > > > > error_mark_node,
> > > > > +     where error_mark_node indicates ill-formed satisfaction.
> > > > > +     It's set to NULL_TREE while computing satisfaction of ATOM+ARGS for
> > > > > +     the first time.  */
> > > > >     tree result;
> > > > > +
> > > > > +  /* The value of input_location when satisfaction of ATOM+ARGS was first
> > > > > +     performed.  */
> > > > > +  location_t location;
> > > > > +
> > > > > +  /* True if this satisfaction result is flagged as "potentially
> > > > > unstable",
> > > > > +     i.e. the result might change at different points in the program if
> > > > > +     recomputed from scratch (which would be ill-formed).  This flag
> > > > > controls
> > > > > +     whether to recompute a cached satisfaction result from scratch even
> > > > > when
> > > > > +     evaluating quietly.  */
> > > > > +  bool maybe_unstable;
> > > > > +
> > > > > +  /* True if we want to diagnose the above instability when it's
> > > > > detected.
> > > > > +     We don't always want to do so, in order to avoid emitting duplicate
> > > > > +     diagnostics in some cases.  */
> > > > > +  bool diagnose_instability;
> > > > >   };
> > > > >     struct sat_hasher : ggc_ptr_hash<sat_entry>
> > > > >   {
> > > > >     static hashval_t hash (sat_entry *e)
> > > > >     {
> > > > > -    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e->constr))
> > > > > +    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e->atom))
> > > > >         {
> > > > >           /* Atoms with instantiated mappings are built during satisfaction.
> > > > >              They live only inside the sat_cache, and we build one to query
> > > > >              the cache with each time we instantiate a mapping.  */
> > > > >           gcc_assert (!e->args);
> > > > > - return hash_atomic_constraint (e->constr);
> > > > > + return hash_atomic_constraint (e->atom);
> > > > >         }
> > > > >         /* Atoms with uninstantiated mappings are built during
> > > > > normalization.
> > > > >          Since normalize_atom caches the atoms it returns, we can assume
> > > > >          pointer-based identity for fast hashing and comparison.  Even if
> > > > > this
> > > > >          assumption is violated, that's okay, we'll just get a cache miss.
> > > > > */
> > > > > -    hashval_t value = htab_hash_pointer (e->constr);
> > > > > +    hashval_t value = htab_hash_pointer (e->atom);
> > > > >   -    if (tree map = ATOMIC_CONSTR_MAP (e->constr))
> > > > > +    if (tree map = ATOMIC_CONSTR_MAP (e->atom))
> > > > >         /* Only the parameters that are used in the targets of the mapping
> > > > >            affect the satisfaction value of the atom.  So we consider only
> > > > >            the arguments for these parameters, and ignore the rest.  */
> > > > > @@ -2425,21 +2472,21 @@ struct sat_hasher : ggc_ptr_hash<sat_entry>
> > > > >       static bool equal (sat_entry *e1, sat_entry *e2)
> > > > >     {
> > > > > -    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->constr)
> > > > > - != ATOMIC_CONSTR_MAP_INSTANTIATED_P (e2->constr))
> > > > > +    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->atom)
> > > > > + != ATOMIC_CONSTR_MAP_INSTANTIATED_P (e2->atom))
> > > > >         return false;
> > > > >         /* See sat_hasher::hash.  */
> > > > > -    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->constr))
> > > > > +    if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->atom))
> > > > >         {
> > > > >           gcc_assert (!e1->args && !e2->args);
> > > > > - return atomic_constraints_identical_p (e1->constr, e2->constr);
> > > > > + return atomic_constraints_identical_p (e1->atom, e2->atom);
> > > > >         }
> > > > >   -    if (e1->constr != e2->constr)
> > > > > +    if (e1->atom != e2->atom)
> > > > >         return false;
> > > > >   -    if (tree map = ATOMIC_CONSTR_MAP (e1->constr))
> > > > > +    if (tree map = ATOMIC_CONSTR_MAP (e1->atom))
> > > > >         for (tree target_parms = TREE_TYPE (map);
> > > > >              target_parms;
> > > > >              target_parms = TREE_CHAIN (target_parms))
> > > > > @@ -2467,59 +2514,146 @@ static GTY((deletable)) hash_table<sat_hasher>
> > > > > *sat_cache;
> > > > >   /* Cache the result of constraint_satisfaction_value.  */
> > > > >   static GTY((deletable)) hash_map<tree, tree> *decl_satisfied_cache;
> > > > >   -static tree
> > > > > -get_satisfaction (tree constr, tree args)
> > > > > -{
> > > > > -  if (!sat_cache)
> > > > > -    return NULL_TREE;
> > > > > -  sat_entry elt = { constr, args, NULL_TREE };
> > > > > -  sat_entry* found = sat_cache->find (&elt);
> > > > > -  if (found)
> > > > > -    return found->result;
> > > > > -  else
> > > > > -    return NULL_TREE;
> > > > > -}
> > > > > -
> > > > > -static void
> > > > > -save_satisfaction (tree constr, tree args, tree result)
> > > > > -{
> > > > > -  if (!sat_cache)
> > > > > -    sat_cache = hash_table<sat_hasher>::create_ggc (31);
> > > > > -  sat_entry elt = {constr, args, result};
> > > > > -  sat_entry** slot = sat_cache->find_slot (&elt, INSERT);
> > > > > -  sat_entry* entry = ggc_alloc<sat_entry> ();
> > > > > -  *entry = elt;
> > > > > -  *slot = entry;
> > > > > -}
> > > > > -
> > > > > -/* A tool to help manage satisfaction caching in satisfy_constraint_r.
> > > > > -   Note the cache is only used when not diagnosing errors.  */
> > > > > +/* A tool used by satisfy_atom to help manage satisfaction caching and to
> > > > > +   diagnose "unstable" satisfaction values.  We insert into the cache
> > > > > only
> > > > > +   when performing satisfaction quietly.  */
> > > > >     struct satisfaction_cache
> > > > >   {
> > > > > -  satisfaction_cache (tree constr, tree args, tsubst_flags_t complain)
> > > > > -    : constr(constr), args(args), complain(complain)
> > > > > -  { }
> > > > > +  satisfaction_cache (tree, tree, sat_info);
> > > > > +  tree get ();
> > > > > +  tree save (tree);
> > > > >   -  tree get ()
> > > > > -  {
> > > > > -    if (complain == tf_none)
> > > > > -      return get_satisfaction (constr, args);
> > > > > -    return NULL_TREE;
> > > > > -  }
> > > > > -
> > > > > -  tree save (tree result)
> > > > > -  {
> > > > > -    if (complain == tf_none)
> > > > > -      save_satisfaction (constr, args, result);
> > > > > -    return result;
> > > > > -  }
> > > > > -
> > > > > -  tree constr;
> > > > > -  tree args;
> > > > > -  tsubst_flags_t complain;
> > > > > +  sat_entry *entry;
> > > > > +  sat_info info;
> > > > > +  unsigned ftc_count;
> > > > >   };
> > > > >   +/* Constructor for the satisfaction_cache class.  We're performing
> > > > > satisfaction
> > > > > +   of ATOM+ARGS according to INFO.  */
> > > > > +
> > > > > +satisfaction_cache
> > > > > +::satisfaction_cache (tree atom, tree args, sat_info info)
> > > > > +  : entry(nullptr), info(info), ftc_count(failed_type_completion_count)
> > > > > +{
> > > > > +  if (!sat_cache)
> > > > > +    sat_cache = hash_table<sat_hasher>::create_ggc (31);
> > > > > +
> > > > > +  /* When noisy, we query the satisfaction cache in order to diagnose
> > > > > +     "unstable" satisfaction values.  */
> > > > > +  if (info.noisy ())
> > > > > +    {
> > > > > +      /* When noisy, constraints have been re-normalized, and that breaks
> > > > > the
> > > > > +  pointer-based identity assumption of sat_cache (for atoms with
> > > > > +  uninstantiated mappings).  So undo this re-normalization by looking
> > > > > in
> > > > > +  the atom_cache for the corresponding atom that was used during quiet
> > > > > +  satisfaction.  */
> > > > > +      if (!ATOMIC_CONSTR_MAP_INSTANTIATED_P (atom))
> > > > > + {
> > > > > +   if (tree found = atom_cache->find (atom))
> > > > > +     atom = found;
> > > > > +   else
> > > > > +     /* The lookup should always succeed, but if it fails then let's
> > > > > +        just leave 'entry' empty, effectively disabling the cache.  */
> > > > > +     return;
> > > > > + }
> > > > > +    }
> > > > > +
> > > > > +  /* Look up or create the corresponding satisfaction entry.  */
> > > > > +  sat_entry elt;
> > > > > +  elt.atom = atom;
> > > > > +  elt.args = args;
> > > > > +  sat_entry **slot = sat_cache->find_slot (&elt, INSERT);
> > > > > +  if (*slot)
> > > > > +    entry = *slot;
> > > > > +  else if (info.quiet ())
> > > > > +    {
> > > > > +      entry = ggc_alloc<sat_entry> ();
> > > > > +      entry->atom = atom;
> > > > > +      entry->args = args;
> > > > > +      entry->result = NULL_TREE;
> > > > > +      entry->location = input_location;
> > > > > +      entry->maybe_unstable = false;
> > > > > +      entry->diagnose_instability = false;
> > > > > +      if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (atom))
> > > > > + /* We always want to diagnose instability of an atom with an
> > > > > +    instantiated parameter mapping.  For atoms with an uninstantiated
> > > > > +    mapping, we set this flag (in satisfy_atom) only if substitution
> > > > > +    into its mapping previously failed.  */
> > > > > + entry->diagnose_instability = true;
> > > > > +      *slot = entry;
> > > > > +    }
> > > > > +  else
> > > > > +    /* We shouldn't get here, but if we do, let's just leave 'entry'
> > > > > +       empty, effectively disabling the cache.  */
> > > > > +    return;
> > > > > +}
> > > > > +
> > > > > +/* Returns the cached satisfaction result if we have one and we're not
> > > > > +   recomputing the satisfaction result from scratch.  Otherwise returns
> > > > > +   NULL_TREE.  */
> > > > > +
> > > > > +tree
> > > > > +satisfaction_cache::get ()
> > > > > +{
> > > > > +  if (!entry)
> > > > > +    return NULL_TREE;
> > > > > +
> > > > > +  if (info.noisy () || entry->maybe_unstable)
> > > > > +    /* We're recomputing the satisfaction result from scratch.  */
> > > > > +    return NULL_TREE;
> > > > > +  else
> > > > > +    return entry->result;
> > > > > +}
> > > > > +
> > > > > +/* RESULT is the computed satisfaction result.  If RESULT differs from
> > > > > the
> > > > > +   previously cached result, this routine issues an appropriate error.
> > > > > +   Otherwise, when evaluating quietly, updates the cache appropriately.
> > > > > */
> > > > > +
> > > > > +tree
> > > > > +satisfaction_cache::save (tree result)
> > > > > +{
> > > > > +  if (!entry)
> > > > > +    return result;
> > > > > +
> > > > > +  if (entry->result && result != entry->result)
> > > > > +    {
> > > > > +      if (info.quiet ())
> > > > > + /* Return error_mark_node to force satisfaction to get replayed
> > > > > +    noisily.  */
> > > > > + return error_mark_node;
> > > > > +      else
> > > > > + {
> > > > > +   if (entry->diagnose_instability)
> > > > > +     {
> > > > > +       auto_diagnostic_group d;
> > > > > +       error_at (EXPR_LOCATION (ATOMIC_CONSTR_EXPR (entry->atom)),
> > > > > +                 "satisfaction value of atomic constraint %qE changed "
> > > > > +                 "from %qE to %qE", entry->atom, entry->result,
> > > > > result);
> > > > > +       inform (entry->location,
> > > > > +               "satisfaction value first evaluated to %qE from here",
> > > > > +               entry->result);
> > > > > +     }
> > > > > +   /* For sake of error recovery, allow this latest satisfaction result
> > > > > +      to prevail.  */
> > > > > +   entry->result = result;
> > > > > +   return result;
> > > > > + }
> > > > > +    }
> > > > > +
> > > > > +  if (info.quiet ())
> > > > > +    {
> > > > > +      entry->result = result;
> > > > > +      /* We heuristically flag this satisfaction result as potentially
> > > > > unstable
> > > > > +  iff during its computation, completion of a type failed.  Note that
> > > > > +  this may also clear the flag if the result turned out to be
> > > > > +  independent of the previously detected type completion failure.  */
> > > > > +      entry->maybe_unstable = (ftc_count !=
> > > > > failed_type_completion_count);
> > > > > +    }
> > > > > +
> > > > > +  return result;
> > > > > +}
> > > > > +
> > > > >   static int satisfying_constraint = 0;
> > > > >     /* Returns true if we are currently satisfying a constraint.
> > > > > @@ -2731,7 +2865,7 @@ static void diagnose_atomic_constraint (tree, tree,
> > > > > tree, subst_info);
> > > > >   static tree
> > > > >   satisfy_atom (tree t, tree args, sat_info info)
> > > > >   {
> > > > > -  satisfaction_cache cache (t, args, info.complain);
> > > > > +  satisfaction_cache cache (t, args, info);
> > > > >     if (tree r = cache.get ())
> > > > >       return r;
> > > > >   @@ -2751,6 +2885,11 @@ satisfy_atom (tree t, tree args, sat_info info)
> > > > >            not satisfied.  Replay the substitution.  */
> > > > >         if (info.diagnose_unsatisfaction_p ())
> > > > >           tsubst_parameter_mapping (ATOMIC_CONSTR_MAP (t), args, info);
> > > > > +      if (info.quiet ())
> > > > > + /* Since instantiation of the parameter mapping failed, we
> > > > > +    want to diagnose potential instability of this satisfaction
> > > > > +    result.  */
> > > > > + cache.entry->diagnose_instability = true;
> > > > >         return cache.save (boolean_false_node);
> > > > >       }
> > > > >   @@ -2762,9 +2901,12 @@ satisfy_atom (tree t, tree args, sat_info info)
> > > > >     ATOMIC_CONSTR_MAP (t) = map;
> > > > >     gcc_assert (!ATOMIC_CONSTR_MAP_INSTANTIATED_P (t));
> > > > >     ATOMIC_CONSTR_MAP_INSTANTIATED_P (t) = true;
> > > > > -  satisfaction_cache inst_cache (t, /*args=*/NULL_TREE, info.complain);
> > > > > +  satisfaction_cache inst_cache (t, /*args=*/NULL_TREE, info);
> > > > >     if (tree r = inst_cache.get ())
> > > > > -    return cache.save (r);
> > > > > +    {
> > > > > +      cache.entry->location = inst_cache.entry->location;
> > > > > +      return cache.save (r);
> > > > > +    }
> > > > >       /* Rebuild the argument vector from the parameter mapping.  */
> > > > >     args = get_mapped_args (map);
> > > > > @@ -2955,6 +3097,8 @@ satisfy_declaration_constraints (tree t, sat_info
> > > > > info)
> > > > >         norm = normalize_nontemplate_requirements (t, info.noisy ());
> > > > >       }
> > > > >   +  unsigned ftc_count = failed_type_completion_count;
> > > > > +
> > > > >     tree result = boolean_true_node;
> > > > >     if (norm)
> > > > >       {
> > > > > @@ -2966,7 +3110,20 @@ satisfy_declaration_constraints (tree t, sat_info
> > > > > info)
> > > > >         pop_tinst_level ();
> > > > >       }
> > > > >   -  if (info.quiet ())
> > > > > +  /* True if this satisfaction is (heuristically) potentially unstable,
> > > > > i.e.
> > > > > +     if its result may depend on where in the program it was performed.
> > > > > */
> > > > > +  bool maybe_unstable_satisfaction = false;
> > > > > +
> > > > > +  if (ftc_count != failed_type_completion_count)
> > > > > +    /* Type completion failure occurred during satisfaction.  The
> > > > > satisfaction
> > > > > +       result may (or may not) materially depend on the completeness of a
> > > > > type,
> > > > > +       so we consider it potentially unstable.   */
> > > > > +    maybe_unstable_satisfaction = true;
> > > > > +
> > > > > +  if (maybe_unstable_satisfaction)
> > > > > +    /* Don't cache potentially unstable satisfaction, to allow
> > > > > satisfy_atom
> > > > > +       to check the stability the next time around.  */;
> > > > > +  else if (info.quiet ())
> > > > >       hash_map_safe_put<hm_ggc> (decl_satisfied_cache, saved_t, result);
> > > > >       return result;
> > > > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > > > > index f59591aa865..209efd4b251 100644
> > > > > --- a/gcc/cp/cp-tree.h
> > > > > +++ b/gcc/cp/cp-tree.h
> > > > > @@ -8075,6 +8075,8 @@ extern hashval_t iterative_hash_constraint
> > > > > (tree, hashval_t);
> > > > >   extern hashval_t hash_atomic_constraint         (tree);
> > > > >   extern void diagnose_constraints                (location_t, tree,
> > > > > tree);
> > > > >   +extern void note_failed_type_completion_for_satisfaction (tree);
> > > > > +
> > > > >   /* A structural hasher for ATOMIC_CONSTRs.  */
> > > > >     struct atom_hasher : default_hash_traits<tree>
> > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > > > > index 2d3ab92dfd1..a2f1fdeb4f5 100644
> > > > > --- a/gcc/cp/pt.c
> > > > > +++ b/gcc/cp/pt.c
> > > > > @@ -15948,13 +15948,8 @@ tsubst (tree t, tree args, tsubst_flags_t
> > > > > complain, tree in_decl)
> > > > >                  But, such constructs have already been resolved by this
> > > > >                  point, so here CTX really should have complete type, unless
> > > > >                  it's a partial instantiation.  */
> > > > > -     ctx = complete_type (ctx);
> > > > > -     if (!COMPLETE_TYPE_P (ctx))
> > > > > -       {
> > > > > -         if (complain & tf_error)
> > > > > -           cxx_incomplete_type_error (NULL_TREE, ctx);
> > > > > -         return error_mark_node;
> > > > > -       }
> > > > > +     if (!complete_type_or_maybe_complain (ctx, NULL_TREE, complain))
> > > > > +       return error_mark_node;
> > > > >             }
> > > > >           f = make_typename_type (ctx, f, typename_type,
> > > > > diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> > > > > index 267b284ea40..d961f719110 100644
> > > > > --- a/gcc/cp/typeck.c
> > > > > +++ b/gcc/cp/typeck.c
> > > > > @@ -154,6 +154,7 @@ complete_type_or_maybe_complain (tree type, tree
> > > > > value, tsubst_flags_t complain)
> > > > >       {
> > > > >         if (complain & tf_error)
> > > > >           cxx_incomplete_type_diagnostic (value, type, DK_ERROR);
> > > > > +      note_failed_type_completion_for_satisfaction (type);
> > > > >         return NULL_TREE;
> > > > >       }
> > > > >     else
> > > > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> > > > > b/gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> > > > > new file mode 100644
> > > > > index 00000000000..e8487bf9c09
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> > > > > @@ -0,0 +1,18 @@
> > > > > +// Verify we diagnose an unstable satisfaction result that depends on
> > > > > +// completeness of the type A below.
> > > > > +//
> > > > > +// { dg-do compile { target c++20 } }
> > > > > +
> > > > > +template <class T> concept has_mem_type = requires { typename T::type; };
> > > > > +// { dg-message "satisfaction of 'has_mem_type<T>' .with T = A." "" {
> > > > > target *-*-* } .-1 }
> > > > > +// { dg-error "satisfaction value of atomic constraint 'requires.typename
> > > > > T::type;. .with T = A.' changed from 'false' to 'true'" "" { target *-*-*
> > > > > } .-2 }
> > > > > +
> > > > > +template <has_mem_type T> int f () { return 0; }
> > > > > +template <class T> char f() { return 0; }
> > > > > +
> > > > > +struct A;
> > > > > +static_assert (sizeof (f<A>()) == 1); // { dg-message "first evaluated to
> > > > > 'false' from here" }
> > > > > +struct A { typedef int type; };
> > > > > +static_assert (sizeof (f<A>()) > 1); // { dg-error "assert" }
> > > > > +// { dg-message "required from here" "" { target *-*-* } .-1 }
> > > > > +static_assert (sizeof (f<A>()) > 1);
> > > > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-complete2.C
> > > > > b/gcc/testsuite/g++.dg/cpp2a/concepts-complete2.C
> > > > > new file mode 100644
> > > > > index 00000000000..b2c11606737
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-complete2.C
> > > > > @@ -0,0 +1,23 @@
> > > > > +// Verify we diagnose an unstable satisfaction result that depends on
> > > > > +// completeness of the type A below.
> > > > > +//
> > > > > +// Like in the previous test, here satisfaction also initially fails,
> > > > > +// but this time due to failed substitution into the atom's parameter
> > > > > mapping.
> > > > > +//
> > > > > +// { dg-do compile { target c++20 } }
> > > > > +
> > > > > +template <class T> concept valid_type = requires { typename T; };
> > > > > +// { dg-message "satisfaction of 'valid_type<typename T::type>' .with T =
> > > > > A." "" { target *-*-* } .-1 }
> > > > > +// { dg-error "satisfaction value of atomic constraint 'requires.T;.
> > > > > .with T = typename T::type.' changed from 'false' to 'true'" "" { target
> > > > > *-*-* } .-2 }
> > > > > +
> > > > > +template <class T> concept has_mem_type = valid_type<typename T::type>;
> > > > > +// { dg-message "satisfaction of 'has_mem_type<T>' .with T = A." "" {
> > > > > target *-*-* } .-1 }
> > > > > +
> > > > > +template <has_mem_type T> int f () { return 0; }
> > > > > +template <class T> char f() { return 0; }
> > > > > +
> > > > > +struct A;
> > > > > +static_assert (sizeof (f<A>()) == 1); // { dg-message "first evaluated to
> > > > > 'false' from here" }
> > > > > +struct A { typedef int type; };
> > > > > +static_assert (sizeof (f<A>()) > 1); // { dg-error "assert" }
> > > > > +static_assert (sizeof (f<A>()) > 1);
> > > > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-complete3.C
> > > > > b/gcc/testsuite/g++.dg/cpp2a/concepts-complete3.C
> > > > > new file mode 100644
> > > > > index 00000000000..5b07371a6be
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-complete3.C
> > > > > @@ -0,0 +1,16 @@
> > > > > +// Verify we diagnose an unstable satisfaction result that depends on
> > > > > +// return type deduction of the member function A::foo() below.
> > > > > +//
> > > > > +// { dg-do compile { target c++20 } }
> > > > > +
> > > > > +template <class T> concept fooable = requires (T t) { t.foo(); };
> > > > > +// { dg-error "'false' to 'true'" "" { target *-*-* } .-1 }
> > > > > +
> > > > > +template <fooable T> int f () { return 0; }
> > > > > +template <class T> char f() { return 0; }
> > > > > +
> > > > > +struct A { auto foo(); };
> > > > > +static_assert (sizeof (f<A>()) == 1); // { dg-message "first evaluated to
> > > > > 'false' from here" }
> > > > > +auto A::foo() { }
> > > > > +static_assert (sizeof (f<A>()) > 1); // { dg-error "assert" }
> > > > > +static_assert (sizeof (f<A>()) > 1);
> > > > > --
> > > > > 2.29.2.404.ge67fbf927d
> > > > >
> > > > >
> > > >
> > >
> > >
> 



More information about the Gcc-patches mailing list