[clang] 79f9cfb - Do not merge LocalInstantiationScope for template specialization

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-06-14T23:28:04-04:00
New Revision: 79f9cfbc21e02555258523ea77c3dd389891cfb3

URL: https://github.com/llvm/llvm-project/commit/79f9cfbc21e02555258523ea77c3dd389891cfb3
DIFF: https://github.com/llvm/llvm-project/commit/79f9cfbc21e02555258523ea77c3dd389891cfb3.diff

LOG: Do not merge LocalInstantiationScope for template specialization

A lambda in a function template may be recursively instantiated. The recursive
lambda will cause a lambda function instantiated multiple times, one inside another.
The inner LocalInstantiationScope should not be marked as MergeWithParentScope
since it already has references to locals properly substituted, otherwise it causes
assertion due to the check for duplicate locals in merged LocalInstantiationScope.

Reviewed by: Richard Smith

Differential Revision: https://reviews.llvm.org/D98068

Added: 
    clang/test/SemaCXX/recursive-lambda.cpp

Modified: 
    clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 8de09def4d52a..b2590d3deaad7 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4890,10 +4890,13 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   // Introduce a new scope where local variable instantiations will be
   // recorded, unless we're actually a member function within a local
   // class, in which case we need to merge our results with the parent
-  // scope (of the enclosing function).
+  // scope (of the enclosing function). The exception is instantiating
+  // a function template specialization, since the template to be
+  // instantiated already has references to locals properly substituted.
   bool MergeWithParentScope = false;
   if (CXXRecordDecl *Rec = dyn_cast<CXXRecordDecl>(Function->getDeclContext()))
-    MergeWithParentScope = Rec->isLocalClass();
+    MergeWithParentScope =
+        Rec->isLocalClass() && !Function->isFunctionTemplateSpecialization();
 
   LocalInstantiationScope Scope(*this, MergeWithParentScope);
 

diff  --git a/clang/test/SemaCXX/recursive-lambda.cpp b/clang/test/SemaCXX/recursive-lambda.cpp
new file mode 100644
index 0000000000000..58087628db988
--- /dev/null
+++ b/clang/test/SemaCXX/recursive-lambda.cpp
@@ -0,0 +1,38 @@
+// 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`. However, since they are not merged, 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>{})
+{
+  constexpr unsigned a = 0;
+  auto f = [&](auto fs, auto i) {
+    if constexpr(i.value > 0)
+    {
+      (void)a;
+      return fs(fs, Number<IBegin>{});
+    }
+    (void)a;
+  };
+
+  return f(f, Number<IEnd>{});
+}
+
+
+void fun2() {
+  fun1();
+}