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