Hi, I'm finally going through the current code on the branch, sorry for the delay.

        * gcc/system.h (cstdlib): Include <cstdlib> to avoid poisoned
        declaration errors.

Poisoned declarations of what? This seems redundant with the #include <stdlib.h> just below.

+  /* Concepts-related keywords */
+  { "assume",          RID_ASSUME,     D_CXXONLY | D_CXX0X | D_CXXWARN },
+  { "axiom",           RID_AXIOM,      D_CXXONLY | D_CXX0X | D_CXXWARN },
+  { "concept",         RID_CONCEPT,    D_CXXONLY | D_CXX0X | D_CXXWARN },
+  { "forall",          RID_FORALL,     D_CXXONLY | D_CXX0X | D_CXXWARN },
+  { "requires",        RID_REQUIRES,   D_CXXONLY | D_CXX0X | D_CXXWARN },

I don't see anything that limits these keywords to when concepts are enabled. You probably want to add an additional mask that applies to these.

+; Activate C++ concepts support.
+Variable
+bool flag_concepts

You don't need to declare this separately.

Components for process constraints and evaluating constraints.

Should that be "processing"?

+// TODO: Simply assinging boolean_type_node to the result type of the 
expression

"assigning"

+// reduced terms in the constraints languaage. Returns NULL_TREE if either A or

"language"

+// a constexpr, nullary function teplate whose result can be converted

"template"

+  // A constraint is declared constexpr

Needs a period.

+// This function is not called for abitrary call expressions. In particul,

"particular"

+static tree
+resolve_constraint_check (tree ovl, tree args)

This function seems to be trying to model a subset of overload resolution, which seems fragile to me; better to use the actual overload resolution code to decide which function the constraint expression calls, or at least resolve_nondeduced_context which handles SFINAE.

+    case CAST_EXPR:
+      return reduce_node (TREE_VALUE (TREE_OPERAND (t, 0)));

Are we assuming that constraint expressions can't involve objects of literal class type?

+// If T is a call to a constraint instantiate it's definition and

"its"

+  tree c = finish_call_expr (t, &args, true, false, 0);
+  error ("invalid requirement");
+  inform (input_location, "did you mean %qE", c);

For both of these diagnostics, let's use EXPR_LOC_OR_HERE (t) as the location.

+// Reduce the requirement T into a logical formula written in terms of
+// atomic propositions.
+tree
+reduce_requirements (tree reqs)

s/T/REQS/

 struct GTY(()) tree_template_info {
   struct tree_common common;
+  tree constraint;
   vec<qualified_typedef_usage_t, va_gc> *typedefs_needing_access_checking;
 };

Why do we need constraint information in template_info? I suppose this is the issue you raised in your mail last month:

In general constraints are directly associated with a template decl. For 
example:

template<Arithmetic T>
  class complex;

The Arithmetic constraint is associated with the template decl. However, this 
doesn't seem to work with partial specializations:

template<Real T>
  struct complex<T> { ... };

I had expected there to be a template decl associated with underlying class, 
but after print-bombing most of the instantiation, lookup, and specialization 
processing routines, I couldn't find that one was ever created for the type 
decl.

This does seem like a shortcoming, that also led to the typedefs vec getting stuck into the template_info inappropriately. I guess we should start building TEMPLATE_DECLs for partial specializations.

+/* Constraint information for a C++ declaration. This includes the
+   requirements (as a constant expression) and the decomposed assumptions
+   and conclusions. The assumptions and conclusions are cached for the
+   purposes of overlaod resolution and diagnostics. */
+struct GTY(()) tree_constraint_info {
+  struct tree_base base;
+  tree spelling;
+  tree requirements;
+  tree assumptions;
+};

I'm confused by the relationship between the comment and the field names here. Where do the conclusions come in? Is "requirements (as a constant expression)" in the spelling or requirements field?

Also, "overload".

+constraint_info_p (tree t)
+template_info_p (tree t)

Let's use check_* rather than *_p for these, too.

+// NODE must be a lang-decl.

Let's say "NODE must have DECL_LANG_SPECIFIC" to avoid confusion with struct lang_decl.

+      error ("concept %q#D declared with function arguments", fn);

s/arguments/parameters/. Some of the gcc internals get this distinction wrong; but we don't need to expose that in diagnostics...

+  // If the concept declaration specifier was found, check
+  // that the declaration satisfies the necessary requirements.
+  if (inlinep & 4)
+    {
+      DECL_DECLARED_CONCEPT_P (decl) = true;
+      if (!check_concept_fn (decl))
+        return NULL_TREE;
+    }

I think I'd rather deal with an invalid concept by not marking it as a concept, but still declaring it as a constexpr function.

+  flag_concepts = true;

This is redundant since c.opt specifies that it defaults to true.

+// Return the current conclision.

"conclusion"

+// Return the current list of assumed terms.
+inline term_list &
+assumptions (proof_state &s) { return assumptions (current_goal (s)); }

Just as a comment, this use of internal iterators and forwarding functions seems confusing; a reader is likely to think that assumptions (s) would produce a list of assumptions for the state as a whole, rather than for the current goal. It's particularly surprising in functions like decompose_left_goal, where we look at the innermost iterator and then pass the outermost object to a subroutine.

+// The following functions are used to manage the insertation and removal
+// goals and terms in the proof state.

"insertion and removal of"

+// the new objet. This does not update the current goal.

"object"

+// And right logical rule.
+inline void
+and_right (proof_state &s)
+{
+  gcc_assert (TREE_CODE (conclusion (s)) == TRUTH_ANDIF_EXPR);
+  tree t = ignore_term (s);
+  conclude_term (branch_goal (s), TREE_OPERAND (t, 1));
+  conclude_term (current_goal (s), TREE_OPERAND (t, 0));
+}

I believe the intent here is to split a sequent

A |- B ^ C

into two,

A |- B
A |- C

right? But since branch_goal updates the current sub-goal, I would expect the result to be

A |-
A |- C, B

instead.  And likewise for or_left.

+// a tree (a vetor of vectors).

"vector"

+// Extract a vector of term vectors from s. The selected set of terms is given
+// by the projection function proj. This is generally either assumptions or
+// conclusions.
+template<typename F>
+  tree
+  extract_goals (proof_state& s, F proj)

Why is proj a function argument rather than a template argument, which would allow inlining?

+// Decompose the requirement R into a set of conclusionss, returning a

"conclusions"

+decompose_conclusions (tree r)

This function is currently never used; I assume that is planned for a later patch?

+// Returns true if the list of assumptions AS subsume the atomic
+// proposition C. This is the case when we can find a proposition in
+// HS that entails the conclusion C.

s/HS/AS/

+      return subsumes_prop (as, TREE_OPERAND (c, 0))
+          && subsumes_prop (as, TREE_OPERAND (c, 1));
+    case TRUTH_ORIF_EXPR:
+      return subsumes_prop (as, TREE_OPERAND (c, 0))
+          || subsumes_prop (as, TREE_OPERAND (c, 1));

Multi-line expressions need parentheses.

+// The following functions will optionally parse a rule if the corresponding
+// criteria is satisfied. If not, the parser returns NULL indicating that
+// the optional parse taken.

Missing a "was not". And these functions should also take the rule as a template argument rather than a function argument. And why do it this way rather than check and possibly return at the top of the function, as elsewhere in the parser? You already have cp_parser_requires_clause checking for RID_REQUIRES.

-        expect the specialization handler to detect and report this.  */
+         expect the specialization handler to detect and report this.  */

There are a bunch of unnecessary whitespace changes in parser.c, including this one.

@@ -19773,6 +19883,17 @@ cp_parser_member_declaration (cp_parser* parser)
              else
                initializer = NULL_TREE;

+              // If we're looking at a function declaration, then a requires
+              // clause may follow the declaration.
+              tree saved_template_reqs = current_template_reqs;

Why don't you use 'release' and conjoin_requirements here?

+// Try to substitute ARGS into PARMS, returning the actual list of
+// arguments that have been substituted. If ARGS cannot be substituted,
+// return error_mark_node.
+tree
+substitute_template_parameters (tree parms, tree args)

The comment sounds more like tsubst_template_parms than coerce_template_parms.

Jason

Reply via email to