This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[c++-concepts] code review


It looks like things are coming together pretty well. What's your feeling about readiness to merge into the trunk? Is the branch down to no regressions?

See you on Monday!

@@ -4146,21 +4146,21 @@ build_new_function_call (tree fn, vec<tree, va_gc> **args, bool koenig_p,
       if (TREE_CODE (fn) == TEMPLATE_ID_EXPR)
         {
           /* If overload resolution selects a specialization of a
+             function concept for non-dependent template arguments,
+             the expression is true if the constraints are satisfied
+             and false otherwise.

              NOTE: This is an extension of Concepts Lite TS that
              allows constraints to be used in expressions. */
+          if (flag_concepts && !processing_template_decl)
             {
               tree tmpl = DECL_TI_TEMPLATE (cand->fn);
+              tree targs = DECL_TI_ARGS (cand->fn);
               tree decl = DECL_TEMPLATE_RESULT (tmpl);
+              if (DECL_DECLARED_CONCEPT_P (decl)
+                  && !uses_template_parms (targs)) {
+                return evaluate_function_concept (decl, targs);

If processing_template_decl is false, uses_template_parms should always be false as well.

+function_concept_check_p (tree t)

+  tree fn = CALL_EXPR_FN (t);
+  if (TREE_CODE (fn) == TEMPLATE_ID_EXPR
+      && TREE_CODE (TREE_OPERAND (fn, 0)) == OVERLOAD)
+    {
+      tree f1 = OVL_FUNCTION (TREE_OPERAND (fn, 0));

I think you want get_first_fn here.

   if (TEMPLATE_PARM_CONSTRAINTS (current_template_parms))
-    TYPE_CANONICAL (type) = type;
+    SET_TYPE_STRUCTURAL_EQUALITY (type);

This seems like papering over an underlying issue. What was the testcase that motivated this change?

@@ -11854,7 +11854,7 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain)
-           if (!spec)
+           if (!spec && DECL_LANG_SPECIFIC (t))
-       if (!local_p)
+       if (!local_p && DECL_LANG_SPECIFIC (r))

What motivated these changes? From the testcase, it seems that you're getting here with the decl for "using TD = int", which shouldn't happen.

@@ -1159,7 +1159,6 @@ check_noexcept_r (tree *tp, int * /*walk_subtrees*/, void * /*data*/)
-      tree type = TREE_TYPE (TREE_TYPE (fn));
-      if (!TYPE_NOTHROW_P (type))
+      if (!TYPE_NOTHROW_P (TREE_TYPE (fn)))

The old code was incorrectly assuming that CALL_EXPR_FN is always a function pointer, but your new code seems to be incorrectly assuming that it's always a function or an expression taking the address of a function; I think this will break on a call to a function pointer variable.

@@ -3481,13 +3481,27 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
     case REQUIRES_EXPR:
+      if (!processing_template_decl)
+        return evaluate_constraint_expression (t, NULL_TREE);
+      else
+        *non_constant_p = true;
+        return t;

We shouldn't get here with a dependent REQUIRES_EXPR (or any dependent expression), so we shouldn't ever hit the else clause.

@@ -18063,18 +18063,41 @@ cp_parser_declarator (cp_parser* parser,
+  /* Function declarations may be followed by a trailing
+     requires-clause. Declarators for function declartions
+     are function declarators wrapping an id-declarator.
+     If the inner declarator is anything else, it does not
+     declare a function. These may also be reference or
+     pointer declarators enclosing such a function declarator.
+     In the declaration :
+
+        int *f(args)
+
+     the declarator is *f(args).
+
+     Abstract declarators cannot have a requires-clauses
+     because they do not declare functions. Here:

         void f() -> int& requires false

+     The trailing return type contains an abstract declarator,
+     and the requires-clause applies to the function
+     declaration and not the abstract declarator.  */
+  if (flag_concepts && dcl_kind != CP_PARSER_DECLARATOR_ABSTRACT)
     {
+      /* We could have things like *f(args) or &f(args).
+         Look inside references and pointers.  */
+      cp_declarator* p = declarator;
+      if (p->kind == cdk_reference || p->kind == cdk_pointer)
+        p = p->declarator;
+
+      /* Pointers or references with no name, or functions
+         with no name cannot have constraints.  */
+      if (!p || !p->declarator)
+        return declarator;
+
+      /* Look for f(args) but not (*f)(args).  */
+      if (p && p->kind == cdk_function && p->declarator->kind == cdk_id)

I think you can use function_declarator_p here.

+static inline bool
+pending_expansion_p (tree t)
+{
+  return (TREE_CODE (t) == PARM_DECL && CONSTRAINT_VAR_P (t)
+          && PACK_EXPANSION_P (TREE_TYPE (t)));
+}

What's the difference between this and function_parameter_pack_p?

+  /* A sentinel class that ensures that deferred access checks
+     are popped before a function returns.  */
+  struct deferring_access_check_sentinel
+  {
+    deferring_access_check_sentinel ()
+    {
+      push_deferring_access_checks (dk_deferred);
+    }
+    ~ deferring_access_check_sentinel ()
+    {
+      pop_deferring_access_checks ();
+    }
+  }

Let's put this with the other RAII sentinels in cp-tree.h.

+                       Lifting of concept definitions

Could we have a bit more description of what "lifting" means here?

+/*---------------------------------------------------------------------------
+                        Constraint normalization
+---------------------------------------------------------------------------*/

You have two of these headers; I guess the first one should be "transformation" and could use more description. On the second, I would retain the old comment

-// Normalize a template requirement to a logical formula written in terms of
-// atomic propositions, returing the new expression.  If the expression cannot
-// be normalized, a NULL_TREE is returned.

+check_implicit_conversion_constraint (tree t, tree args,
+                                      tsubst_flags_t complain, tree in_decl)
+{
+  tree expr = ICONV_CONSTR_EXPR (t);
+
+  /* Don't tsubst as if we're processing a template. If we try
+     to we can end up generating template-like expressions
+     (e.g., modop-exprs) that aren't properly typed. */
+  int saved_template_decl = processing_template_decl;
+  processing_template_decl = 0;

Why are we checking constraints when processing_template_decl is true?

+   Note that this is the only place that we instantiate the
+   constraints. */
+bool
+check_constraints (tree ci, tree args)

Except for tsubst_constraint?

+  ++processing_template_decl;
+  tree constr = transform_expression (lift_function_definition (fn, args));
+  --processing_template_decl;

Why do you need to set processing_template_decl here and in other calls to transform_expression? I don't notice anything that would be helped, especially now that you're using separate tree codes for constraints, though there is this in check_logical_expr:

+  /* Resolve the logical operator. Note that template processing is
+     disabled so we get the actual call or target expression back.
+     not_processing_template_sentinel sentinel.

I guess that isn't needed anymore?

+   FIXME: This is defined in pt.c because it's garbage collection
+   code is not being generated for constraint.cc. */
+static GTY (()) hash_table<constr_hasher> *decl_constraints;

I think you need to add constraint.cc to the gtfiles variable in cp/config-lang.in. It also looks like the GTFILES_H rule in gcc/Makefile.in assumes that all sources have the .c extension, so that may need adjustment.

@@ -2454,6 +2454,8 @@ finish_call_expr (tree fn, vec<tree, va_gc> **args, bool d
              next = OVL_CHAIN (fn);
+              if (flag_concepts)
+                remove_constraints (OVL_FUNCTION (fn));

I don't think we want to remove constraints here; this code is just discarding the OVERLOAD node, not the FUNCTION_DECL.

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]