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 3285 Build 3285: 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 | ||
---|---|---|
476–499 | 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. | |
476–499 | 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. |
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).