[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 8 inline comments as done.
erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:4705
       CheckConstraintSatisfaction(
-          NamedConcept, {NamedConcept->getConstraintExpr()}, Converted,
+          NamedConcept, {NamedConcept->getConstraintExpr()}, MLTAL,
           SourceRange(SS.isSet() ? SS.getBeginLoc() : ConceptNameInfo.getLoc(),
----------------
ChuanqiXu wrote:
> I would feel better if we could write:
> ```
> CheckConstraintSatisfaction(
>           NamedConcept, {NamedConcept->getConstraintExpr()}, {MLTAL},
>           SourceRange(SS.isSet() ? SS.getBeginLoc() : ConceptNameInfo.getLoc(),
>                       TemplateArgs->getRAngleLoc()),
>           Satisfaction)
> ```
> 
> But it looks unimplementable.
I'm not sure I get the suggestion?  Why would you want to put the `MultiLevelTemplateArgumentList` in curleys?  


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:7468-7483
+      // Attn Reviewers: This works and fixes the constraint comparison issues,
+      // but I don't have a good idea why this is, nor if it is the 'right'
+      // thing.  I THINK it is pulling off the 'template template' level of the
+      // constraint, but I have no idea if that is the correct thing to do.
+      SmallVector<const Expr *, 3> ConvertedParamsAC;
+      TemplateArgumentList Empty(TemplateArgumentList::OnStack, {});
+      MultiLevelTemplateArgumentList MLTAL{Empty};
----------------
ChuanqiXu wrote:
> I've spent some time to playing it myself to figure it out. And I found that we could fix this cleaner we adopt above suggestions. The problem here is that the instantiation is started half way. But the conversion for the constraint have been deferred. So here is the problem. I guess there are other similar problems. But let's fix them after we met them actually.
> 
Ah, neat!  Thanks for this.  Done.


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

https://reviews.llvm.org/D119544