[PATCH] D119544: Deferred Concept Instantiation Implementation

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.
This revision is now accepted and ready to land.

LGTM basically. Please wait for 1~2 weeks to land this in case there are other comments.



================
Comment at: clang/lib/Sema/SemaConcept.cpp:496-497
+
+  // Attn Reviewers: we need to do this for the function constraints for
+  // comparison of constraints to work, but do we also need to do it for
+  // CheckInstantiatedFunctionConstraints?  That one is more difficult, but we
----------------
We need to edit the `Attn Reviewers` to `TODO` before landing. The content need to be rewording too. Your English is much better than me. So no concrete suggestion here : )


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:4705
       CheckConstraintSatisfaction(
-          NamedConcept, {NamedConcept->getConstraintExpr()}, Converted,
+          NamedConcept, {NamedConcept->getConstraintExpr()}, MLTAL,
           SourceRange(SS.isSet() ? SS.getBeginLoc() : ConceptNameInfo.getLoc(),
----------------
erichkeane wrote:
> 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?  
I just feel like the style is more cleaner. But I found the constructor might not allow us to do so... So this one might not be a suggestion.


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

https://reviews.llvm.org/D119544