On Fri, 22 May 2020, Jason Merrill wrote:
> On 5/22/20 9:18 PM, Patrick Palka wrote:
> > On Fri, 22 May 2020, Jason Merrill wrote:
> > > On 5/20/20 10:08 PM, Patrick Palka wrote:
> > > > On Wed, 20 May 2020, Patrick Palka wrote:
> > > > > On Tue, 19 May 2020, Jason Merrill wrote:
> > > > > 
> > > > > > On 5/8/20 11:42 AM, Patrick Palka wrote:
> > > > > > > On Wed, 6 May 2020, Patrick Palka wrote:
> > > > > > > 
> > > > > > > > On Wed, 6 May 2020, Patrick Palka wrote:
> > > > > > > > 
> > > > > > > > > On Tue, 5 May 2020, Patrick Palka wrote:
> > > > > > > > > 
> > > > > > > > > > On Tue, 5 May 2020, Patrick Palka wrote:
> > > > > > > > > > 
> > > > > > > > > > > Unfortunately, the previous fix to PR94038 is fragile.
> > > > > > > > > > > When
> > > > > > > > > > > the
> > > > > > > > > > > argument to fold_for_warn is a bare CALL_EXPR, then all is
> > > > > > > > > > > well: the
> > > > > > > > > > > result of maybe_constant_value from fold_for_warn (with
> > > > > > > > > > > uid_sensitive=true) is reused via the cv_cache in the
> > > > > > > > > > > subsequent
> > > > > > > > > > > call to
> > > > > > > > > > > maybe_constant_value from cp_fold (with
> > > > > > > > > > > uid_sensitive=false),
> > > > > > > > > > > so we
> > > > > > > > > > > avoid instantiating bar<int>.
> > > > > > > > > > > 
> > > > > > > > > > > But when the argument to fold_for_warn is more complex,
> > > > > > > > > > > e.g.
> > > > > > > > > > > an
> > > > > > > > > > > INDIRECT_REF of a CALL_EXPR, as in the testcase below (due
> > > > > > > > > > > to
> > > > > > > > > > > bar<int>()
> > > > > > > > > > > returning const int& which we need to decay to int) then
> > > > > > > > > > > from
> > > > > > > > > > > fold_for_warn we call maybe_constant_value on the
> > > > > > > > > > > INDIRECT_REF, and
> > > > > > > > > > > from
> > > > > > > > > > > cp_fold we call it on the CALL_EXPR, so there is no reuse
> > > > > > > > > > > via
> > > > > > > > > > > the
> > > > > > > > > > > cv_cache and we therefore end up instantiating bar<int>.
> > > > > > > > > > > 
> > > > > > > > > > > So for a more robust solution to this general issue of
> > > > > > > > > > > warning
> > > > > > > > > > > flags
> > > > > > > > > > > affecting code generation, it seems that we need a way to
> > > > > > > > > > > globally
> > > > > > > > > > > avoid
> > > > > > > > > > > template instantiation during constexpr evaluation
> > > > > > > > > > > whenever
> > > > > > > > > > > we're
> > > > > > > > > > > performing warning-dependent folding.
> > > > > > > > > > > 
> > > > > > > > > > > To that end, this patch replaces the flag
> > > > > > > > > > > constexpr_ctx::uid_sensitive
> > > > > > > > > > > with a global flag uid_sensitive_constexpr_evaluation_p,
> > > > > > > > > > > and
> > > > > > > > > > > enables
> > > > > > > > > > > it
> > > > > > > > > > > during fold_for_warn using an RAII helper.
> > > > > > > > > > > 
> > > > > > > > > > > The patch also adds a counter that keeps track of the
> > > > > > > > > > > number
> > > > > > > > > > > of
> > > > > > > > > > > times
> > > > > > > > > > > uid_sensitive_constexpr_evaluation_p is called, and we use
> > > > > > > > > > > this to
> > > > > > > > > > > determine whether the result of constexpr evaluation
> > > > > > > > > > > depended
> > > > > > > > > > > on the
> > > > > > > > > > > flag's value.  This lets us safely update the cv_cache and
> > > > > > > > > > > fold_cache
> > > > > > > > > > > from fold_for_warn in the most common case where the
> > > > > > > > > > > flag's
> > > > > > > > > > > value
> > > > > > > > > > > was
> > > > > > > > > > > irrelevant during constexpr evaluation.
> > > > > > > 
> > > > > > > Here's some statistics about about the fold cache and cv cache
> > > > > > > that
> > > > > > > were
> > > > > > > collected when building the testsuite of range-v3 (with the
> > > > > > > default
> > > > > > > build flags which include -O -Wall -Wextra, so warning-dependent
> > > > > > > folding
> > > > > > > applies).
> > > > > > > 
> > > > > > > The "naive solution" below refers to the one in which we would
> > > > > > > refrain
> > > > > > > from updating the caches at the end of
> > > > > > > cp_fold/maybe_constant_value
> > > > > > > whenever we're in uid-sensitive constexpr evaluation mode (i.e.
> > > > > > > whenever
> > > > > > > we're called from fold_for_warn), regardless of whether constexpr
> > > > > > > evaluation was actually restricted.
> > > > > > > 
> > > > > > > 
> > > > > > > FOLD CACHE  | cache hit | cache miss    | cache miss        |
> > > > > > >                |           | cache updated | cache not updated |
> > > > > > > ------------+-----------+---------------+-------------------+
> > > > > > > naive sol'n |   5060779 |      11374667 |          12887790 |
> > > > > > > ------------------------------------------------------------+
> > > > > > > this patch  |   6665242 |      19688975 |            199137 |
> > > > > > > ------------+-----------+---------------+-------------------+
> > > > > > > 
> > > > > > > 
> > > > > > > CV CACHE    | cache hit | cache miss    | cache miss        |
> > > > > > >                |           | cache updated | cache not updated |
> > > > > > > ------------+-----------+---------------+-------------------+
> > > > > > > naive sol'n |     76214 |       3637218 |           6889213 |
> > > > > > > ------------------------------------------------------------+
> > > > > > > this patch  |    215980 |       9882310 |            405513 |
> > > > > > > ------------+-----------+---------------+-------------------+
> > > > > > > 
> > > > > > > -- >8 --
> > > > > > > 
> > > > > > > gcc/cp/ChangeLog:
> > > > > > > 
> > > > > > >   PR c++/94038
> > > > > > >   * constexpr.c (constexpr_ctx::uid_sensitive): Remove field.
> > > > > > >   (uid_sensitive_constexpr_evaluation_value): Define.
> > > > > > >   (uid_sensitive_constexpr_evaluation_true_counter): Define.
> > > > > > >   (uid_sensitive_constexpr_evaluation_p): Define.
> > > > > > >   (uid_sensitive_constexpr_evaluation_sentinel): Define its
> > > > > > >   constructor.
> > > > > > >   (uid_sensitive_constexpr_evaluation_checker): Define its
> > > > > > >   constructor and its evaluation_restricted_p method.
> > > > > > >   (get_fundef_copy): Remove 'ctx' parameter.  Use u_s_c_e_p
> > > > > > >   instead of constexpr_ctx::uid_sensitive.
> > > > > > >   (cxx_eval_call_expression): Use u_s_c_e_p instead, and test it
> > > > > > >   last.  Adjust call to get_fundef_copy.
> > > > > > >   (instantiate_cx_fn_r): Test u_s_c_e_p so that we increment the
> > > > > > >   counter if necessary.
> > > > > > >   (cxx_eval_outermost_constant_expr): Remove 'uid_sensitive'
> > > > > > >   parameter.  Adjust function body accordingly.
> > > > > > >   (maybe_constant_value): Remove 'uid_sensitive' parameter and
> > > > > > >   adjust function body accordingly.  Set up a
> > > > > > >   uid_sensitive_constexpr_evaluation_checker, and use it to
> > > > > > >   conditionally update the cv_cache.
> > > > > > >   * cp-gimplify.h (cp_fold): Set up a
> > > > > > >   uid_sensitive_constexpr_evaluation_checker, and use it to
> > > > > > >   conditionally update the fold_cache.
> > > > > > >   * cp-tree.h (maybe_constant_value): Update declaration.
> > > > > > >   (struct uid_sensitive_constexpr_evaluation_sentinel): Define.
> > > > > > >   (struct sensitive_constexpr_evaluation_checker): Define.
> > > > > > >   * expr.c (fold_for_warn): Set up a
> > > > > > >   uid_sensitive_constexpr_evaluation_sentinel before calling
> > > > > > >   the folding subroutines.  Drop all but the first argument to
> > > > > > >   maybe_constant_value.
> > > > > > > 
> > > > > > > gcc/testsuite/ChangeLog:
> > > > > > > 
> > > > > > >   PR c++/94038
> > > > > > >   * g++.dg/warn/pr94038-2.C: New test.
> > > > > > > ---
> > > > > > >     gcc/cp/constexpr.c                    | 92
> > > > > > > ++++++++++++++++++++-------
> > > > > > >     gcc/cp/cp-gimplify.c                  | 13 ++--
> > > > > > >     gcc/cp/cp-tree.h                      | 23 ++++++-
> > > > > > >     gcc/cp/expr.c                         |  4 +-
> > > > > > >     gcc/testsuite/g++.dg/warn/pr94038-2.C | 28 ++++++++
> > > > > > >     5 files changed, 130 insertions(+), 30 deletions(-)
> > > > > > >     create mode 100644 gcc/testsuite/g++.dg/warn/pr94038-2.C
> > > > > > > 
> > > > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > > > > > > index 706d8a13d8e..d2b75ddaa19 100644
> > > > > > > --- a/gcc/cp/constexpr.c
> > > > > > > +++ b/gcc/cp/constexpr.c
> > > > > > > @@ -1089,11 +1089,59 @@ struct constexpr_ctx {
> > > > > > >       bool strict;
> > > > > > >       /* Whether __builtin_is_constant_evaluated () should be
> > > > > > > true.  */
> > > > > > >       bool manifestly_const_eval;
> > > > > > > -  /* Whether we want to avoid doing anything that will cause
> > > > > > > extra
> > > > > > > DECL_UID
> > > > > > > -     generation.  */
> > > > > > > -  bool uid_sensitive;
> > > > > > >     };
> > > > > > >     +/* This internal flag controls whether we should avoid doing
> > > > > > > anything
> > > > > > > during
> > > > > > > +   constexpr evaluation that would cause extra DECL_UID
> > > > > > > generation,
> > > > > > > such as
> > > > > > > +   template instantiation and function copying.  */
> > > > > > > +
> > > > > > > +static bool uid_sensitive_constexpr_evaluation_value;
> > > > > > > +
> > > > > > > +/* An internal counter that keeps track of the number of times
> > > > > > > +   uid_sensitive_constexpr_evaluation_p returned true.  */
> > > > > > > +
> > > > > > > +static unsigned uid_sensitive_constexpr_evaluation_true_counter;
> > > > > > > +
> > > > > > > +/* The accessor for uid_sensitive_constexpr_evaluation_value
> > > > > > > which
> > > > > > > also
> > > > > > > +   increments the corresponding counter.  */
> > > > > > > +
> > > > > > > +static bool
> > > > > > > +uid_sensitive_constexpr_evaluation_p ()
> > > > > > > +{
> > > > > > > +  if (uid_sensitive_constexpr_evaluation_value)
> > > > > > > +    {
> > > > > > > +      ++uid_sensitive_constexpr_evaluation_true_counter;
> > > > > > > +      return true;
> > > > > > > +    }
> > > > > > > +  else
> > > > > > > +    return false;
> > > > > > > +}
> > > > > > > +
> > > > > > > +uid_sensitive_constexpr_evaluation_sentinel
> > > > > > > +::uid_sensitive_constexpr_evaluation_sentinel ()
> > > > > > > +  : ovr (uid_sensitive_constexpr_evaluation_value, true)
> > > > > > > +{
> > > > > > > +}
> > > > > > > +
> > > > > > > +uid_sensitive_constexpr_evaluation_checker
> > > > > > > +::uid_sensitive_constexpr_evaluation_checker ()
> > > > > > > +  : saved_counter
> > > > > > > (uid_sensitive_constexpr_evaluation_true_counter)
> > > > > > > +{
> > > > > > > +}
> > > > > > 
> > > > > > These two constructors need comments.
> > > > > > 
> > > > > > > +/* Returns true iff uid_sensitive_constexpr_evaluation_p is true,
> > > > > > > +   and some constexpr evaluation was restricted due to u_s_c_e_p
> > > > > > > +   being called and returning true after this checker object was
> > > > > > > +   constructed.  */
> > > > > > > +
> > > > > > > +bool
> > > > > > > +uid_sensitive_constexpr_evaluation_checker::evaluation_restricted_p
> > > > > > > ()
> > > > > > > const
> > > > > > > +{
> > > > > > > +  return (uid_sensitive_constexpr_evaluation_value
> > > > > > > +   && saved_counter !=
> > > > > > > uid_sensitive_constexpr_evaluation_true_counter);
> > > > > > > +}
> > > > > > > +
> > > > > > > +
> > > > > > >     /* A table of all constexpr calls that have been evaluated by
> > > > > > > the
> > > > > > >        compiler in this translation unit.  */
> > > > > > >     @@ -1156,7 +1204,7 @@ static GTY(()) hash_map<tree, tree>
> > > > > > > *fundef_copies_table;
> > > > > > >        is parms, TYPE is result.  */
> > > > > > >       static tree
> > > > > > > -get_fundef_copy (const constexpr_ctx *ctx, constexpr_fundef
> > > > > > > *fundef)
> > > > > > > +get_fundef_copy (constexpr_fundef *fundef)
> > > > > > >     {
> > > > > > >       tree copy;
> > > > > > >       bool existed;
> > > > > > > @@ -1173,7 +1221,7 @@ get_fundef_copy (const constexpr_ctx *ctx,
> > > > > > > constexpr_fundef *fundef)
> > > > > > >         }
> > > > > > >       else if (*slot == NULL_TREE)
> > > > > > >         {
> > > > > > > -      if (ctx->uid_sensitive)
> > > > > > > +      if (uid_sensitive_constexpr_evaluation_p ())
> > > > > > >           return NULL_TREE;
> > > > > > >             /* We've already used the function itself, so make a
> > > > > > > copy.
> > > > > > > */
> > > > > > > @@ -2292,8 +2340,8 @@ cxx_eval_call_expression (const
> > > > > > > constexpr_ctx
> > > > > > > *ctx,
> > > > > > > tree t,
> > > > > > >         /* We can't defer instantiating the function any longer.
> > > > > > > */
> > > > > > >       if (!DECL_INITIAL (fun)
> > > > > > > -      && !ctx->uid_sensitive
> > > > > > > -      && DECL_TEMPLOID_INSTANTIATION (fun))
> > > > > > > +      && DECL_TEMPLOID_INSTANTIATION (fun)
> > > > > > > +      && !uid_sensitive_constexpr_evaluation_p ())
> > > > > > >         {
> > > > > > >           location_t save_loc = input_location;
> > > > > > >           input_location = loc;
> > > > > > > @@ -2454,7 +2502,7 @@ cxx_eval_call_expression (const
> > > > > > > constexpr_ctx
> > > > > > > *ctx,
> > > > > > > tree t,
> > > > > > >             gcc_assert (at_eof >= 2 && ctx->quiet);
> > > > > > >             *non_constant_p = true;
> > > > > > >           }
> > > > > > > -      else if (tree copy = get_fundef_copy (ctx,
> > > > > > > new_call.fundef))
> > > > > > > +      else if (tree copy = get_fundef_copy (new_call.fundef))
> > > > > > >           {
> > > > > > >             tree body, parms, res;
> > > > > > >             releasing_vec ctors;
> > > > > > > @@ -6485,7 +6533,8 @@ instantiate_cx_fn_r (tree *tp, int
> > > > > > > *walk_subtrees,
> > > > > > > void */*data*/)
> > > > > > >           && DECL_DECLARED_CONSTEXPR_P (*tp)
> > > > > > >           && !DECL_INITIAL (*tp)
> > > > > > >           && !trivial_fn_p (*tp)
> > > > > > > -      && DECL_TEMPLOID_INSTANTIATION (*tp))
> > > > > > > +      && DECL_TEMPLOID_INSTANTIATION (*tp)
> > > > > > > +      && !uid_sensitive_constexpr_evaluation_p ())
> > > > > > >         {
> > > > > > >           ++function_depth;
> > > > > > >           instantiate_decl (*tp, /*defer_ok*/false,
> > > > > > > /*expl_inst*/false);
> > > > > > > @@ -6552,8 +6601,7 @@ cxx_eval_outermost_constant_expr (tree t,
> > > > > > > bool
> > > > > > > allow_non_constant,
> > > > > > >                                     bool strict = true,
> > > > > > >                                     bool manifestly_const_eval =
> > > > > > > false,
> > > > > > >                                     bool constexpr_dtor = false,
> > > > > > > -                           tree object = NULL_TREE,
> > > > > > > -                           bool uid_sensitive = false)
> > > > > > > +                           tree object = NULL_TREE)
> > > > > > >     {
> > > > > > >       auto_timevar time (TV_CONSTEXPR);
> > > > > > >     @@ -6569,8 +6617,7 @@ cxx_eval_outermost_constant_expr (tree
> > > > > > > t,
> > > > > > > bool
> > > > > > > allow_non_constant,
> > > > > > >       constexpr_global_ctx global_ctx;
> > > > > > >       constexpr_ctx ctx = { &global_ctx, NULL, NULL, NULL, NULL,
> > > > > > > NULL,
> > > > > > > NULL,
> > > > > > >                           allow_non_constant, strict,
> > > > > > > -                 manifestly_const_eval || !allow_non_constant,
> > > > > > > -                 uid_sensitive };
> > > > > > > +                 manifestly_const_eval || !allow_non_constant
> > > > > > > };
> > > > > > >         tree type = initialized_type (t);
> > > > > > >       tree r = t;
> > > > > > > @@ -6660,8 +6707,7 @@ cxx_eval_outermost_constant_expr (tree t,
> > > > > > > bool
> > > > > > > allow_non_constant,
> > > > > > >       auto_vec<tree, 16> cleanups;
> > > > > > >       global_ctx.cleanups = &cleanups;
> > > > > > >     -  if (!uid_sensitive)
> > > > > > > -    instantiate_constexpr_fns (r);
> > > > > > > +  instantiate_constexpr_fns (r);
> > > > > > >       r = cxx_eval_constant_expression (&ctx, r,
> > > > > > >                                       false, &non_constant_p,
> > > > > > > &overflow_p);
> > > > > > >     @@ -6909,14 +6955,12 @@ fold_simple (tree t)
> > > > > > >        Otherwise, if T does not have TREE_CONSTANT set, returns T.
> > > > > > >        Otherwise, returns a version of T without TREE_CONSTANT.
> > > > > > >        MANIFESTLY_CONST_EVAL is true if T is manifestly
> > > > > > > const-evaluated
> > > > > > > -   as per P0595.  UID_SENSITIVE is true if we can't do anything
> > > > > > > that
> > > > > > > -   would affect DECL_UID ordering.  */
> > > > > > > +   as per P0595.  */
> > > > > > >       static GTY((deletable)) hash_map<tree, tree> *cv_cache;
> > > > > > >       tree
> > > > > > > -maybe_constant_value (tree t, tree decl, bool
> > > > > > > manifestly_const_eval,
> > > > > > > -               bool uid_sensitive)
> > > > > > > +maybe_constant_value (tree t, tree decl, bool
> > > > > > > manifestly_const_eval)
> > > > > > >     {
> > > > > > >       tree r;
> > > > > > >     @@ -6934,8 +6978,7 @@ maybe_constant_value (tree t, tree decl,
> > > > > > > bool
> > > > > > > manifestly_const_eval,
> > > > > > >         return t;
> > > > > > >         if (manifestly_const_eval)
> > > > > > > -    return cxx_eval_outermost_constant_expr (t, true, true, true,
> > > > > > > false,
> > > > > > > -                                      decl, uid_sensitive);
> > > > > > > +    return cxx_eval_outermost_constant_expr (t, true, true, true,
> > > > > > > false,
> > > > > > > decl);
> > > > > > >         if (cv_cache == NULL)
> > > > > > >         cv_cache = hash_map<tree, tree>::create_ggc (101);
> > > > > > > @@ -6950,14 +6993,15 @@ maybe_constant_value (tree t, tree decl,
> > > > > > > bool
> > > > > > > manifestly_const_eval,
> > > > > > >           return r;
> > > > > > >         }
> > > > > > >     -  r = cxx_eval_outermost_constant_expr (t, true, true, false,
> > > > > > > false,
> > > > > > > -                                 decl, uid_sensitive);
> > > > > > > +  uid_sensitive_constexpr_evaluation_checker c;
> > > > > > > +  r = cxx_eval_outermost_constant_expr (t, true, true, false,
> > > > > > > false,
> > > > > > > decl);
> > > > > > >       gcc_checking_assert (r == t
> > > > > > >                          || CONVERT_EXPR_P (t)
> > > > > > >                          || TREE_CODE (t) == VIEW_CONVERT_EXPR
> > > > > > >                          || (TREE_CONSTANT (t) && !TREE_CONSTANT
> > > > > > > (r))
> > > > > > >                          || !cp_tree_equal (r, t));
> > > > > > > -  cv_cache->put (t, r);
> > > > > > > +  if (!c.evaluation_restricted_p ())
> > > > > > > +    cv_cache->put (t, r);
> > > > > > >       return r;
> > > > > > >     }
> > > > > > >     diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
> > > > > > > index fc26a85f43a..f298fa7cec1 100644
> > > > > > > --- a/gcc/cp/cp-gimplify.c
> > > > > > > +++ b/gcc/cp/cp-gimplify.c
> > > > > > > @@ -2443,6 +2443,8 @@ cp_fold (tree x)
> > > > > > >       if (tree *cached = fold_cache->get (x))
> > > > > > >         return *cached;
> > > > > > >     +  uid_sensitive_constexpr_evaluation_checker c;
> > > > > > > +
> > > > > > >       code = TREE_CODE (x);
> > > > > > >       switch (code)
> > > > > > >         {
> > > > > > > @@ -2925,10 +2927,13 @@ cp_fold (tree x)
> > > > > > >           return org_x;
> > > > > > >         }
> > > > > > >     -  fold_cache->put (org_x, x);
> > > > > > > -  /* Prevent that we try to fold an already folded result again.
> > > > > > > */
> > > > > > > -  if (x != org_x)
> > > > > > > -    fold_cache->put (x, x);
> > > > > > > +  if (!c.evaluation_restricted_p ())
> > > > > > > +    {
> > > > > > > +      fold_cache->put (org_x, x);
> > > > > > > +      /* Prevent that we try to fold an already folded result
> > > > > > > again.
> > > > > > > */
> > > > > > > +      if (x != org_x)
> > > > > > > + fold_cache->put (x, x);
> > > > > > > +    }
> > > > > > >         return x;
> > > > > > >     }
> > > > > > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > > > > > > index deb000babc1..c2511196de4 100644
> > > > > > > --- a/gcc/cp/cp-tree.h
> > > > > > > +++ b/gcc/cp/cp-tree.h
> > > > > > > @@ -7949,7 +7949,7 @@ extern bool
> > > > > > > require_potential_rvalue_constant_expression (tree);
> > > > > > >     extern tree cxx_constant_value                        (tree, 
> > > > > > > tree =
> > > > > > > NULL_TREE);
> > > > > > >     extern void cxx_constant_dtor                 (tree, tree);
> > > > > > >     extern tree cxx_constant_init                 (tree, tree =
> > > > > > > NULL_TREE);
> > > > > > > -extern tree maybe_constant_value         (tree, tree =
> > > > > > > NULL_TREE, bool
> > > > > > > = false, bool = false);
> > > > > > > +extern tree maybe_constant_value         (tree, tree =
> > > > > > > NULL_TREE, bool
> > > > > > > = false);
> > > > > > >     extern tree maybe_constant_init                       (tree, 
> > > > > > > tree =
> > > > > > > NULL_TREE, bool = false);
> > > > > > >     extern tree fold_non_dependent_expr           (tree,
> > > > > > >                                                    tsubst_flags_t
> > > > > > > =
> > > > > > > tf_warning_or_error,
> > > > > > > @@ -7968,6 +7968,27 @@ extern tree fold_sizeof_expr
> > > > > > > (tree);
> > > > > > >     extern void clear_cv_and_fold_caches          (bool = true);
> > > > > > >     extern tree unshare_constructor                       (tree
> > > > > > > CXX_MEM_STAT_INFO);
> > > > > > >     +/* An RAII sentinel used to restrict constexpr evaluation so
> > > > > > > that
> > > > > > > it
> > > > > > > +   doesn't do anything that causes extra DECL_UID generation.  */
> > > > > > > +
> > > > > > > +struct uid_sensitive_constexpr_evaluation_sentinel
> > > > > > > +{
> > > > > > > +  temp_override<bool> ovr;
> > > > > > > +  uid_sensitive_constexpr_evaluation_sentinel ();
> > > > > > > +};
> > > > > > > +
> > > > > > > +/* Used to determine whether uid_sensitive_constexpr_evaluation_p
> > > > > > > was
> > > > > > > +   called and returned true, indicating that we've restricted
> > > > > > > constexpr
> > > > > > > +   evaluation in order to avoid UID generation.  We use this to
> > > > > > > control
> > > > > > > +   updates to the fold_cache and cv_cache.  */
> > > > > > > +
> > > > > > > +struct uid_sensitive_constexpr_evaluation_checker
> > > > > > > +{
> > > > > > > +  const unsigned saved_counter;
> > > > > > > +  uid_sensitive_constexpr_evaluation_checker ();
> > > > > > > +  bool evaluation_restricted_p () const;
> > > > > > > +};
> > > > > > > +
> > > > > > >     /* In cp-ubsan.c */
> > > > > > >     extern void cp_ubsan_maybe_instrument_member_call (tree);
> > > > > > >     extern void cp_ubsan_instrument_member_accesses (tree *);
> > > > > > > diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
> > > > > > > index 9b535708c57..0d68a738f8f 100644
> > > > > > > --- a/gcc/cp/expr.c
> > > > > > > +++ b/gcc/cp/expr.c
> > > > > > > @@ -399,6 +399,8 @@ fold_for_warn (tree x)
> > > > > > >     {
> > > > > > >       /* C++ implementation.  */
> > > > > > >     +  uid_sensitive_constexpr_evaluation_sentinel s;
> > > > > > 
> > > > > > Please add a comment about why we need this.
> > > > > 
> > > > > Thanks for the review.  Here's the final patch (with the added
> > > > > comments)
> > > > > that I've committed:
> > > 
> > > This seems to have broken cmcstl2:
> > > 
> > > > Building CXX object
> > > > test/iterator/CMakeFiles/make_range.dir/make_range.cpp.o
> > > > In file included from
> > > > /home/jason/src/cmcstl2/include/stl2/type_traits.hpp:16,
> > > >                   from
> > > > /home/jason/src/cmcstl2/include/stl2/iterator.hpp:18,
> > > >                   from
> > > > /home/jason/src/cmcstl2/test/iterator/make_range.cpp:1:
> > > > /home/jason/s/gcc/x86_64-pc-linux-gnu/libstdc++-v3/include/type_traits:
> > > > In
> > > > instantiation of ‘struct std::is_nothrow_destructible<int*>’:
> > > > /home/jason/s/gcc/x86_64-pc-linux-gnu/libstdc++-v3/include/type_traits:3170:35:
> > > > required from ‘constexpr const bool
> > > > std::is_nothrow_destructible_v<int*>’
> > > > /home/jason/src/cmcstl2/include/stl2/detail/concepts/object/move_constructible.hpp:34:35:
> > > > required from here
> > > > /home/jason/s/gcc/x86_64-pc-linux-gnu/libstdc++-v3/include/type_traits:895:52:
> > > > error: non-constant condition for static assertion
> > > >    895 |
> > > > static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
> > > >        |
> > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
> > > > /home/jason/s/gcc/x86_64-pc-linux-gnu/libstdc++-v3/include/type_traits:895:52:
> > > > error: ‘constexpr std::true_type
> > > > std::__is_complete_or_unbounded(std::__type_identity<_Tp>) [with _Tp =
> > > > int*;
> > > > long unsigned int <anonymous> = 8; std::true_type =
> > > > std::integral_constant<bool, true>]’ used before its definition
> > 
> > Yikes :( The following fixlet seems to restore the cmcstl2 build:
> > 
> > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > index 98c974e657f..4e441ac8d2f 100644
> > --- a/gcc/cp/constexpr.c
> > +++ b/gcc/cp/constexpr.c
> > @@ -6486,7 +6486,8 @@ cxx_eval_constant_expression (const constexpr_ctx
> > *ctx, tree t,
> >         break;
> >       }
> >   - if (!processing_template_decl)
> > +   if (!processing_template_decl
> > +       && !uid_sensitive_constexpr_evaluation_p ())
> >       r = evaluate_concept_check (t, tf_warning_or_error);
> >     else
> >       *non_constant_p = true;
> > 
> > Concept evaluation may entail DECL_UID generation and/or template
> > instantiation, so in general it seems we can't perform it during
> > uid-sensitive constexpr evaluation.
> 
> Makes sense.  The patch is OK.

I've just committed the following after bootstrap/regtest on
x86_64-pc-linux-gnu, and after verifying that the build of cmcstl2 is
restored:

-- >8 --

Subject: [PATCH] c++: Avoid concept evaluation when uid-sensitive [PR94038]

Concept evaluation may entail DECL_UID generation and/or template
instantiation, so in general we can't perform it during uid-sensitive
constexpr evaluation.

gcc/cp/ChangeLog:

        PR c++/94038
        * constexpr.c (cxx_eval_constant_expression)
        <case TEMPLATE_ID_EXPR>: Don't evaluate the concept when
        constexpr evaluation is uid-sensitive.

gcc/testsuite/ChangeLog:

        PR c++/94038
        * g++.dg/warn/pr94038-3.C: New test.
---
 gcc/cp/constexpr.c                    |  3 ++-
 gcc/testsuite/g++.dg/warn/pr94038-3.C | 15 +++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/pr94038-3.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 98c974e657f..4e441ac8d2f 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -6486,7 +6486,8 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
            break;
          }
 
-       if (!processing_template_decl)
+       if (!processing_template_decl
+           && !uid_sensitive_constexpr_evaluation_p ())
          r = evaluate_concept_check (t, tf_warning_or_error);
        else
          *non_constant_p = true;
diff --git a/gcc/testsuite/g++.dg/warn/pr94038-3.C 
b/gcc/testsuite/g++.dg/warn/pr94038-3.C
new file mode 100644
index 00000000000..49b6d133f08
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr94038-3.C
@@ -0,0 +1,15 @@
+// PR c++/94038
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-Wall" }
+
+template<typename T>
+constexpr int foo() {
+  return T::x;
+}
+
+constexpr bool bar(bool a) { return a; }
+
+template<typename T>
+concept C = foo<T>() == 0;
+
+static_assert(decltype(bar(C<int>)){} == false);
-- 
2.27.0.rc1.5.gae92ac8ae3

Reply via email to