[PATCH] D44352: [Concepts] Constrained template parameters

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.

This needs revision to reflect changes between the Concepts TS and C++20. In particular, only the name of a //type-concept// can be used to declare a //template-parameter// in the new rules:

  template<typename> concept A = true;
  template<template<typename> typename> B = true;
  template<int> concept C = true;
  
  template<A> struct XA {}; // ok
  template<B> struct XB {}; // error, not a type-concept
  template<C> struct XC {}; // error, not a type-concept

We don't appear to have any explicit representation of the //type-concept// as written, only of its immediately-declared constraint. One design goal of Clang is for the AST to directly represent the program as written (to the extent possible) to facilitate writing tools on top of Clang's AST. Please consider adding a representation of the `TypeConstraint` itself. (This could be as simple as a wrapper around the `Expr*` that you currently have, that provides access to the concept name and the template arguments, and additionally stores a flag to determine whether an explicit template argument list was specified, to distinguish `TypeConcept` from `TypeConcept<>`.)



================
Comment at: lib/Parse/ParseTemplate.cpp:590-619
+///         constrained-parameter
+///
+///       type-parameter: (See below)
+///         type-parameter-key ...[opt] identifier[opt]
+///         type-parameter-key identifier[opt] = type-id
+///         'template' '<' template-parameter-list '>' type-parameter-key
+///               ...[opt] identifier[opt]
----------------
This is out of date compared to the current C++20 wording.


================
Comment at: lib/Parse/ParseTemplate.cpp:677
+                                          TemplateArgumentListInfo &TALI) {
+  TentativeParsingAction TPA(*this);
+
----------------
This should not require tentative parsing. We should be able to annotate the nested-name-specifier (if any) then determine whether we have a //type-name// or a //type-constraint// with a single token of lookahead.


================
Comment at: lib/Parse/ParseTemplate.cpp:1039-1045
+    if (!EllipsisLoc.isInvalid()
+        && CD->getTemplateParameters()->hasParameterPack())
+      // C++ [temp.param]p11.1
+      //   If P declares a template parameter pack and C is a variadic concept,
+      //   then A is the pack expansion P... . Otherwise, A is the
+      //   id-expression P.
+      Q = Actions.Context.getPackExpansionType(Q, /*NumExpansions=*/None);
----------------
This is incorrect; this rule was removed when concepts were merged into C++20.


================
Comment at: lib/Parse/ParseTemplate.cpp:1153-1176
+  Expr *IntroducedConstraint = Result.get();
+  if (EllipsisLoc.isValid() && !CD->getTemplateParameters()->hasParameterPack())
+    // We have the following case:
+    //
+    // template<typename T> concept C1 = true;
+    // template<C1... T> struct s1;
+    //
----------------
All the work to build expressions and establish the semantic effects of declarations should be done by `Sema`, not by the `Parser`. The `Parser` should just figure out how the program matches the grammar, and report to `Sema` what it found.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D44352/new/

https://reviews.llvm.org/D44352