[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2431-2439
+  //if (TrailingRequiresClause) {
+  //  Sema::CXXThisScopeRAII ThisScope(SemaRef, Record, D->getMethodQualifiers());
+  //  ExprResult Rebuilt =
+  //      SemaRef.RebuildExprInCurrentInstantiation(TrailingRequiresClause);
+  //  if (Rebuilt.isInvalid())
+  //    return nullptr;
+  //  TrailingRequiresClause = Rebuilt.get();
----------------
ChuanqiXu wrote:
> The failing test case could be fixed by the above change. The change above is just a hack instead of a suggestion.
> The rationale is that we need to record some information when we traverse the template decl. The bugs now looks like we forget to do something which would be done at the first time. And we meet in problems due to the missed information (we couldn't find the member in the class).
So this would un-do the 'deferred constraint checking', because that woudl cause us to immediately instantiate the template, so I don't think we want to do that.


================
Comment at: clang/test/SemaTemplate/concepts.cpp:386-388
+  []()
+    requires(constraint<decltype(Local)>)
+  {}();
----------------
ChuanqiXu wrote:
> We might need more negative tests.
> Now it would pass even if I write:
> ```
> []()
>     requires(false)
> {}();
> ```
Ouch!  I'll have to see if I can find where we're missing that check!


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

https://reviews.llvm.org/D119544