[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I went to commit this, and found that a recently lit test now fails with a crash during constraint instantiation!  I'll be looking into that.  The example reduces to:

  template<typename T>
  constexpr bool constraint = true;
   
  template <typename T, typename U>
  void dependent(U&& u) {
    []() requires constraint<decltype(u)> {}();
  }
   
  void test_dependent() {
    int v   = 0;
    dependent<int&>(v);
  }



================
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:
> 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.
Ah, you mean to pass 'converted' directly in, so:
```
CheckConstraintSatisfaction(
          NamedConcept, {NamedConcept->getConstraintExpr()}, {Converted},
          SourceRange(SS.isSet() ? SS.getBeginLoc() : ConceptNameInfo.getLoc(),
                      TemplateArgs->getRAngleLoc()),
          Satisfaction)
```

(notice 'Converted' instead of MLTAL).  I agree with you, that WOULD be nicer, but unfortunately I think the constructor for that type was created explicitly to avoid us doing that :)


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

https://reviews.llvm.org/D119544