This adds associated constraints as a property of class templates.
An error is produced if redeclarations are not similarly constrained.
Details
Diff Detail
- Build Status
Buildable 3832 Build 3832: arc lint + arc unit
Event Timeline
| test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp | ||
|---|---|---|
| 13 | I should probably add some template-dependent cases. | |
| include/clang/AST/DeclTemplate.h | ||
|---|---|---|
| 456–479 | 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). | |
| lib/Sema/SemaTemplate.cpp | ||
| 1178 | I'm not a big fan of continue as a proxy for a goto. It looks like you could rearrange the logic here to avoid this: bool ConstraintMismatch = false;
if ((bool)CurAC != (bool)PrevAC)
ConstraintMismatch = true;
else if (CurAC) {
// profile and set ConstraintMismatch
}
if (ConstraintMismatch) Diag(...)... or factor out a lambda to do the mismatch checking. | |
| test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp | ||
| 2 | I like the idea of separate subdirectories of test/CXX for TSes. Thanks :) | |
Address review comments; update to revision 292996
Fix possibly ill-formed NDR case
Test template-dependent cases for class redeclaration
Address review comment: use lambda instead of do while(0)
Address review comment: use PointerUnion w/struct
| include/clang/AST/DeclTemplate.h | ||
|---|---|---|
| 355 | Removed this constructor as a drive-by. | |
| 456–479 | 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. | |
| test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp | ||
| 10 | 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. | |
| 13 | Propagation of associated constraints to instantiated specializations will need to be added later. | |
| include/clang/AST/DeclTemplate.h | ||
|---|---|---|
| 373–391 | This mechanism seems unnecessary to me; allocating the ConstrainedTemplateDeclInfo separately seems a lot simpler. Forcing this and the template into a single allocation is unlikely to help anything since we use a slab allocator (which is going to lay the objects out the same way this template trick does, unless we hit the end of a slab). | |
| lib/Sema/SemaTemplate.cpp | ||
| 1178 | [nit] Comments should be full sentences: capitalized and ending in a period. | |
| 1299–1300 | Is there a reason you don't want to store the associated constraints that were specified on a redeclaration? I'd expect that to hurt tools that want source fidelity (for instance, a renaming tool will want to be able to find all the references to a particular name, even in a requires-clause on a redeclaration of a template). | |
| include/clang/AST/DeclTemplate.h | ||
|---|---|---|
| 373–391 | Okay; I'll probably allocate separately in the Create function. | |
| lib/Sema/SemaTemplate.cpp | ||
| 1178 | Okay. | |
| 1299–1300 | Associated constraints are not part of the source fidelity: the requires-clause on the template-parameter-list is (and later, if constraints are introduced by abbreviated function call syntax, etc., the function parameter list). | |
Address review comments; update to revision 294580
Allocate ConstrainedTemplateDeclInfo separately
Update comments to be sentences; NFC
| lib/Sema/SemaTemplate.cpp | ||
|---|---|---|
| 1300 | Add period to this sentence. | |
Removed this constructor as a drive-by.