Added code to correctly calculate the associated constraints of a template (no enforcement yet). Depends on D41217.
Rather than producing an Expr* (which will presumably eventually need to contain synthetic && expressions), this should produce a vector of constraints (notionally and'd together), which will (eventually) be either constraint-expressions or some representation of "the constraints implied by this template parameter". If you do that, I don't think there's any reason to cache this information on the TemplateDecl; it should be cheap enough to recompute when we need it.
|47–58 ↗||(On Diff #159236)|
I'm sure we already have this "profile two expressions and check they are canonically equivalent" functionality factored out somewhere. Is there nothing like that that's appropriate to reuse here?
Unrelated cleanup: please commit this separately.
This is wrong (we should never substitute into a constraint-expression except as part of satisfaction or subsumption checks). Please at least add a FIXME.
There doesn't seem to be a good reason to serialize the cached associated constraints, even if we want to keep them (which I don't think we do).
What would be left "to do" here? For constrained parameters, there may be something interesting to track on the *Template*ParmDecl, but not here, I wouldn't think,
For my own edification, could you explain whether, given
#define BOOL bool using typedef_for_bool = bool;
you'd expect to diagnose a redeclaration of B::A with associated constraint
requires bool( U() ) // extra whitespace
requires BOOL(U()) // different spelling of `bool`
requires typedef_for_bool(U()) // different spelling of `bool`
? My naive reading of N4762 temp.constr.atomic/2 says that none of these constraints (on line 10) would be "identical" to the constraint on line 6... but then I don't understand what's the salient difference between line 10 (which apparently gives no error) and line 22 (which apparently gives an error).
I saw the !... but I don't understand how the compiler "knows" that !bool(U()) is "different" from bool(T()) in a way that doesn't equally apply to bool(U()).
Or suppose the constraint on line 10 was requires bool(U())==true... would that give a diagnostic?
bool(T()) and bool(U()) are identical because they have the same parameter mappings.
The "identical" requirement applies to the actual grammar composition of the expression, so bool(T()) would be different to bool(T()) == true.
At least that's how I understand it.
OK, I can see where the confusion is coming from.
The way it works (in clang, at least) - is that the compiler pays no attention to the name of a template parameter for any purpose other than actually finding it in the first place - once it is found, it is 'stored' simply as bool(<template-parameter-0-0>()) where the first 0 is the depth of the template parameter list of the parameter in question (in case of a template within a template) and the second 0 is the index of the template parameter within that list.
I believe this treatment stems from [temp.over.link]p6 "When determining whether types or qualified-concept-names are equivalent, the rules above are used to compare expressions involving template parameters"
Okay, I can see how this matches the apparent intent of http://eel.is/c++draft/temp.over.link#5 . I guess a strict reading of that passage would mean that my BOOL and typedef_for_bool versions should give diagnostics, and so should
#define V(x) U template <typename U> requires bool(V(x) ())
but no diagnostic is expected for
#define V U template <typename U> requires bool(V ())
Anyway, sounds like this rabbit hole is out-of-scope for this review, anyway, so I'll be quiet now. :)
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.
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 TemplateParameterLists, profile them, and compare the results.
There's no such thing any more.
There's no such thing any more.