[PATCH] D119544: Deferred Concept Instantiation Implementation

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D119544#3472537 <https://reviews.llvm.org/D119544#3472537>, @erichkeane wrote:

> Alright, a touch more:  It looks like the looping through the `CurContext` at line 6084 finds the wrong thing in evaluating this `CXXMethodDecl`!  The "broken" version has its CurContext as `bar` in the example above, the "working" versions have `foo` or `foo2<int>`.  Therefore the 2nd time through the loop, those both end up on the `ClassTemplateSpecializationDecl`, but the broken one finds the `isFileContext` version.
>
> So I don't know HOW to fix it, but it seems that the `CheckFunctionConstraints` probably has to change the `CurContext`.  I suspect the instantiation of the `WorkingRequires` does something like this correctly.
>
> I'm poking on/off on it today, but if @ChuanqiXu has an idea/help, it would be greatly appreciated.

Yeah, I also find the information looks in a mess. I spent some time to look at the bug (my workaround shows below) but it would make deferred-concept-inst.cpp fail... it would say `foo()` is not a member of `Test`... I don't have strong proves here. But I guess we might need to look at tree transformation for RequireExpr in `TreeTransform<Derived>::TransformRequiresExpr`.



================
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();
----------------
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).


================
Comment at: clang/test/SemaTemplate/concepts.cpp:386-388
+  []()
+    requires(constraint<decltype(Local)>)
+  {}();
----------------
We might need more negative tests.
Now it would pass even if I write:
```
[]()
    requires(false)
{}();
```


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

https://reviews.llvm.org/D119544