[PATCH] D25674: [Concepts] Class template associated constraints

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast marked 3 inline comments as done.
hubert.reinterpretcast added inline comments.


================
Comment at: include/clang/AST/DeclTemplate.h:355
 protected:
-  // This is probably never used.
-  TemplateDecl(Kind DK, DeclContext *DC, SourceLocation L, DeclarationName Name)
----------------
Removed this constructor as a drive-by.


================
Comment at: include/clang/AST/DeclTemplate.h:412-417
+  /// \brief The template parameter list and optional requires-clause
+  /// associated with this declaration.
+  ///
+  /// The boolean value indicates whether this particular declaration has an
+  /// attached \c Expr representing the associated constraints of the template.
+  llvm::PointerIntPair<TemplateParameterList *, 1, bool> TemplateParams;
----------------
rsmith wrote:
> Rather than tail-allocating the associated constraints (which will be an inconvenience for every class that derives from `TemplateDecl`), can you store this as a `PointerUnion` of a `TemplateParameterList*` and a pointer to a struct wrapping a `TemplateParameterList*` and the associated constraints expression? I'm not really concerned about paying one extra pointer per associated constraints expression we store (and I suspect we may want to store this as something richer than an expression at some point).
As discussed in person, I am using `PointerUnion` in the base class. I am still allocating the proposed struct alongside the `TemplateDecl` instance where it is not too much trouble.


================
Comment at: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp:9
+struct A;
+template <typename T> requires !!sizeof(MyBool)
+struct A;
----------------
Status of this line is unclear. A conservative user of the language should probably avoid this as ill-formed NDR (functionally equivalent but not equivalent). This line as been replaced.


================
Comment at: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp:12
+
+} // end namespace nodiag
+
----------------
hubert.reinterpretcast wrote:
> I should probably add some template-dependent cases.
Propagation of associated constraints to instantiated specializations will need to be added later.


https://reviews.llvm.org/D25674