[PATCH] D41284: [Concepts] Associated constraints infrastructure.

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

rsmith added inline comments.


================
Comment at: include/clang/AST/DeclTemplate.h:177
+  /// conjunction ("and").
+  llvm::SmallVector<const Expr *, 3> getAssociatedConstraints() const;
+
----------------
Returning a `SmallVector` by value is not idiomatic. If you need to make a copy here, you should take a `SmallVectorImpl` by reference, so the caller can choose the inline size. If not, you should return an `ArrayRef`.


================
Comment at: include/clang/Sema/Sema.h:5772-5776
+  /// \brief Check that the associated constraints of a template declaration
+  /// match the associated constraints of an older declaration of which it is a
+  /// redeclaration
+  bool CheckRedeclarationConstraintMatch(ArrayRef<const Expr *> OldAC,
+                                         ArrayRef<const Expr *> NewAC);
----------------
This is not how redeclaration checking is specified any more. A redeclaration is valid only if its //template-head// is equivalent to that of the prior declaration, not if it has the same associated constraints. You should instead take two `TemplateParameterList`s, profile them, and compare the results.


================
Comment at: lib/Serialization/ASTReaderDecl.cpp:2375
     D->ParameterPack = Record.readInt();
+    // TODO: Concepts: Constrained parameters
     if (Record.readInt())
----------------
There's no such thing any more.


================
Comment at: lib/Serialization/ASTReaderDecl.cpp:2393
     // Rest of TemplateTemplateParmDecl.
+    // TODO: Concepts: Constrained parameters
     D->ParameterPack = Record.readInt();
----------------
There's no such thing any more.


Repository:
  rC Clang

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

https://reviews.llvm.org/D41284