[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

Oh, the diff part is small really. The change looks good to me and I've tested it for libcxx.

But it crashed at "clang/lib/Sema/SemaTemplateInstantiate.cpp:1852: clang::QualType {anonymous}::TemplateInstantiator::TransformTemplateTypeParmType(clang::TypeLocBuilder&, clang::TemplateTypeParmTypeLoc): Assertion `Arg.getKind() == TemplateArgument::Type && "Template argument kind mismatch"' failed". I am trying to get a reduced example. I would suggest to do more testing with other workloads (with libstdc++ >= 10)



================
Comment at: clang/lib/Sema/SemaConcept.cpp:442-444
+    }
+  } else if (FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization ||
+             FD->getTemplatedKind() == FunctionDecl::TK_DependentNonTemplate) {
----------------
nit: This is not important but I prefer the style. It makes each logical section shorter and indentation shorter.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:1699
+      : inherited(SemaRef), TemplateDepth(TemplateDepth) {}
+  bool getResult() { return Result; }
+
----------------



================
Comment at: clang/lib/Sema/SemaTemplate.cpp:1701
+
+  // As far as I can tell, this should be the only template parm type that we
+  // have to deal with.  SubstTempalteTypeParmPack,
----------------
We shouldn't use `I`.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:1717
+  Decl *TransformDecl(SourceLocation Loc, Decl *D) {
+    // FIXME : This is an incomplete list, but I'm not sure of how each of the
+    // Decl kinds could be used to refer to the template parameters.  This is my
----------------
We shouldn't use `I` in comments since other readers don't know what `I` refers to. (Maybe they could use git blame after all)


================
Comment at: clang/test/SemaTemplate/concepts-friends.cpp:1
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
----------------
Maybe it is better to add `-fsyntax-only`


================
Comment at: clang/test/SemaTemplate/deferred-concept-inst.cpp:1
+// RUN: %clang_cc1 -std=c++2a -x c++ %s -verify -Wno-unused-value
+// expected-no-diagnostics
----------------
Maybe it is better to add `-fsyntax-only`


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

https://reviews.llvm.org/D126907