yaxunl updated this revision to Diff 351934.
yaxunl retitled this revision from "Remove asserts for LocalInstantiationScope" 
to "Exampt asserts for recursive lambdas about LocalInstantiationScope".
yaxunl edited the summary of this revision.
yaxunl added a comment.

re-enable the assert but with exception for recursive lambda functions


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

https://reviews.llvm.org/D98068

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/SemaCXX/recursive-lambda.cpp


Index: clang/test/SemaCXX/recursive-lambda.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/recursive-lambda.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// Check recursive instantiation of lambda does not cause assertion.
+// lambda function `f` in `fun1` is instantiated twice: first
+// as f(f, Number<1>), then as f(f, Number<0>). The
+// LocalInstantiationScopes of these two instantiations both contain
+// `f` and `i`. Clang should not assert for that.
+
+template <unsigned v>
+struct Number
+{
+   static constexpr unsigned value = v;
+};
+
+template <unsigned IBegin = 0,
+          unsigned IEnd = 1>
+constexpr auto fun1(Number<IBegin> = Number<0>{}, Number<IEnd>  = Number<1>{})
+{
+  auto f = [&](auto fs, auto i) {
+    if constexpr(i.value > 0)
+    {
+      return fs(fs, Number<IBegin>{});
+    }
+  };
+
+  return f(f, Number<IEnd>{});
+}
+
+
+void fun2() {
+  fun1();
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3624,11 +3624,13 @@
   llvm::PointerUnion<Decl *, DeclArgumentPack *> &Stored = LocalDecls[D];
   if (Stored.isNull()) {
 #ifndef NDEBUG
-    // It should not be present in any surrounding scope either.
+    // It should not be present in any surrounding scope either except for
+    // recursive lambda functions.
     LocalInstantiationScope *Current = this;
     while (Current->CombineWithOuterScope && Current->Outer) {
       Current = Current->Outer;
-      assert(Current->LocalDecls.find(D) == Current->LocalDecls.end() &&
+      assert((isLambdaCallOperator(D->getDeclContext()) ||
+              Current->LocalDecls.find(D) == Current->LocalDecls.end()) &&
              "Instantiated local in inner and outer scopes");
     }
 #endif
@@ -3649,10 +3651,12 @@
 
 void LocalInstantiationScope::MakeInstantiatedLocalArgPack(const Decl *D) {
 #ifndef NDEBUG
-  // This should be the first time we've been told about this decl.
+  // This should be the first time we've been told about this decl except for
+  // recursive lambda functions.
   for (LocalInstantiationScope *Current = this;
        Current && Current->CombineWithOuterScope; Current = Current->Outer)
-    assert(Current->LocalDecls.find(D) == Current->LocalDecls.end() &&
+    assert((isLambdaCallOperator(D->getDeclContext()) ||
+            Current->LocalDecls.find(D) == Current->LocalDecls.end()) &&
            "Creating local pack after instantiation of local");
 #endif
 


Index: clang/test/SemaCXX/recursive-lambda.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/recursive-lambda.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// Check recursive instantiation of lambda does not cause assertion.
+// lambda function `f` in `fun1` is instantiated twice: first
+// as f(f, Number<1>), then as f(f, Number<0>). The
+// LocalInstantiationScopes of these two instantiations both contain
+// `f` and `i`. Clang should not assert for that.
+
+template <unsigned v>
+struct Number
+{
+   static constexpr unsigned value = v;
+};
+
+template <unsigned IBegin = 0,
+          unsigned IEnd = 1>
+constexpr auto fun1(Number<IBegin> = Number<0>{}, Number<IEnd>  = Number<1>{})
+{
+  auto f = [&](auto fs, auto i) {
+    if constexpr(i.value > 0)
+    {
+      return fs(fs, Number<IBegin>{});
+    }
+  };
+
+  return f(f, Number<IEnd>{});
+}
+
+
+void fun2() {
+  fun1();
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3624,11 +3624,13 @@
   llvm::PointerUnion<Decl *, DeclArgumentPack *> &Stored = LocalDecls[D];
   if (Stored.isNull()) {
 #ifndef NDEBUG
-    // It should not be present in any surrounding scope either.
+    // It should not be present in any surrounding scope either except for
+    // recursive lambda functions.
     LocalInstantiationScope *Current = this;
     while (Current->CombineWithOuterScope && Current->Outer) {
       Current = Current->Outer;
-      assert(Current->LocalDecls.find(D) == Current->LocalDecls.end() &&
+      assert((isLambdaCallOperator(D->getDeclContext()) ||
+              Current->LocalDecls.find(D) == Current->LocalDecls.end()) &&
              "Instantiated local in inner and outer scopes");
     }
 #endif
@@ -3649,10 +3651,12 @@
 
 void LocalInstantiationScope::MakeInstantiatedLocalArgPack(const Decl *D) {
 #ifndef NDEBUG
-  // This should be the first time we've been told about this decl.
+  // This should be the first time we've been told about this decl except for
+  // recursive lambda functions.
   for (LocalInstantiationScope *Current = this;
        Current && Current->CombineWithOuterScope; Current = Current->Outer)
-    assert(Current->LocalDecls.find(D) == Current->LocalDecls.end() &&
+    assert((isLambdaCallOperator(D->getDeclContext()) ||
+            Current->LocalDecls.find(D) == Current->LocalDecls.end()) &&
            "Creating local pack after instantiation of local");
 #endif
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D98068: E... Yaxun Liu via Phabricator via cfe-commits

Reply via email to