Merge pull request #476 from andreasfertig/fixIssue467

Fixed #467: Forward from invoke to call operator

Author: andreasfertig

CodeGenerator.cpp

@@ -978,34 +978,56 @@ void CodeGenerator::InsertArg(const VarDecl* stmt)
 
 bool CodeGenerator::InsertLambdaStaticInvoker(const CXXMethodDecl* cxxMethodDecl)
 {
-    if(cxxMethodDecl && cxxMethodDecl->isLambdaStaticInvoker()) {
-        mOutputFormatHelper.AppendNewLine();
+    if(not(cxxMethodDecl and cxxMethodDecl->isLambdaStaticInvoker())) {
+        return false;
+    }
 
-        const auto* lambda = cxxMethodDecl->getParent();
-        const auto* callOp = lambda->getLambdaCallOperator();
-        if(lambda->isGenericLambda() && cxxMethodDecl->isFunctionTemplateSpecialization()) {
-            const TemplateArgumentList* tal            = cxxMethodDecl->getTemplateSpecializationArgs();
-            FunctionTemplateDecl*       callOpTemplate = callOp->getDescribedFunctionTemplate();
-            void*                       insertPos      = nullptr;
-            FunctionDecl*               correspondingCallOpSpecialization =
-                callOpTemplate->findSpecialization(tal->asArray(), insertPos);
-            callOp = cast<CXXMethodDecl>(correspondingCallOpSpecialization);
-        }
+    // A special case for a lambda with a static invoker. The standard says, that in such a case invoking the call
+    // operator gives the same result as invoking the function pointer (see [expr.prim.lambda.closure] p9). When it
+    // comes to block local statics having a body for both functions reveals a difference. This special code
+    // generates a forwarding call from the call operator to the static invoker. However, the compiler does better
+    // here. As this way we end up with copies of the parameters which is hard to avoid.
 
-        InsertArg(callOp->getBody());
-        mOutputFormatHelper.AppendNewLine();
+    mOutputFormatHelper.AppendNewLine();
+    mOutputFormatHelper.OpenScope();
 
-        return true;
+    if(not cxxMethodDecl->getReturnType()->isVoidType()) {
+        mOutputFormatHelper.Append(kwReturn, " "sv);
     }
 
-    return false;
+    mOutputFormatHelper.Append(GetName(*cxxMethodDecl->getParent()), "{}.operator()");
+
+    if(cxxMethodDecl->isFunctionTemplateSpecialization()) {
+        InsertTemplateArgs(*dyn_cast_or_null<FunctionDecl>(cxxMethodDecl));
+    }
+
+    if(cxxMethodDecl->isTemplated()) {
+        if(cxxMethodDecl->getDescribedTemplate()) {
+            InsertTemplateParameters(*cxxMethodDecl->getDescribedTemplate()->getTemplateParameters(),
+                                     TemplateParamsOnly::Yes);
+        }
+        /*else if(decl.isFunctionTemplateSpecialization()) {
+            InsertTemplateSpecializationHeader();
+        }*/
+    }
+
+    WrapInParens([&] {
+        mOutputFormatHelper.AppendParameterList(cxxMethodDecl->parameters(),
+                                                OutputFormatHelper::NameOnly::Yes,
+                                                OutputFormatHelper::GenMissingParamName::Yes);
+    });
+
+    mOutputFormatHelper.AppendSemiNewLine();
+    mOutputFormatHelper.CloseScope(OutputFormatHelper::NoNewLineBefore::Yes);
+    mOutputFormatHelper.AppendNewLine();
+
+    return true;
 }
 //-----------------------------------------------------------------------------
 
 /// \brief Inserts the instantiation point of a template.
 //
 // This reveals at which place the template is first used.
-
 void CodeGenerator::InsertInstantiationPoint(const SourceManager&  sm,
                                              const SourceLocation& instLoc,
                                              std::string_view      text)
@@ -1172,9 +1194,16 @@ static std::string GetTypeConstraintAsString(const TypeConstraint* typeConstrain
 }
 //-----------------------------------------------------------------------------
 
-void CodeGenerator::InsertTemplateParameters(const TemplateParameterList& list)
+void CodeGenerator::InsertTemplateParameters(const TemplateParameterList& list,
+                                             const TemplateParamsOnly     templateParamsOnly)
 {
-    mOutputFormatHelper.Append(kwTemplate, "<"sv);
+    const bool full{TemplateParamsOnly::No == templateParamsOnly};
+
+    if(full) {
+        mOutputFormatHelper.Append(kwTemplate);
+    }
+
+    mOutputFormatHelper.Append("<"sv);
 
     OnceFalse needsComma{};
     for(const auto* param : list) {
@@ -1183,18 +1212,24 @@ void CodeGenerator::InsertTemplateParameters(const TemplateParameterList& list)
         const auto& typeName = GetName(*param);
 
         if(const auto* tt = dyn_cast_or_null<TemplateTypeParmDecl>(param)) {
-            if(tt->wasDeclaredWithTypename()) {
-                mOutputFormatHelper.Append(kwTypeNameSpace);
-            } else if(not tt->hasTypeConstraint()) {
-                mOutputFormatHelper.Append(kwClassSpace);
-            }
+            if(full) {
+                if(tt->wasDeclaredWithTypename()) {
+                    mOutputFormatHelper.Append(kwTypeNameSpace);
+                } else if(not tt->hasTypeConstraint()) {
+                    mOutputFormatHelper.Append(kwClassSpace);
+                }
 
-            if(tt->isParameterPack()) {
-                mOutputFormatHelper.Append(kwElipsisSpace);
+                if(tt->isParameterPack()) {
+                    mOutputFormatHelper.Append(kwElipsisSpace);
+                }
             }
 
             if(0 == typeName.size() || tt->isImplicit() /* fixes class container:auto*/) {
-                AppendTemplateTypeParamName(mOutputFormatHelper, tt, false);
+                AppendTemplateTypeParamName(mOutputFormatHelper, tt, not full);
+
+                if(not full and tt->isParameterPack()) {
+                    mOutputFormatHelper.Append(kwElipsis);
+                }
 
             } else {
                 if(auto typeConstraint = GetTypeConstraintAsString(tt->getTypeConstraint());
@@ -1232,9 +1267,12 @@ void CodeGenerator::InsertTemplateParameters(const TemplateParameterList& list)
         }
     }
 
-    mOutputFormatHelper.AppendNewLine(">"sv);
+    mOutputFormatHelper.Append(">"sv);
 
-    InsertConceptConstraint(list);
+    if(full) {
+        mOutputFormatHelper.AppendNewLine();
+        InsertConceptConstraint(list);
+    }
 }
 //-----------------------------------------------------------------------------
 
@@ -3742,6 +3780,10 @@ void CodeGenerator::InsertFunctionNameWithReturnType(const FunctionDecl&       d
     const bool  isClassTemplateSpec{isCXXMethodDecl && isa<ClassTemplateSpecializationDecl>(methodDecl->getParent())};
     const bool  requiresComment{isCXXMethodDecl && not methodDecl->isUserProvided() &&
                                not methodDecl->isExplicitlyDefaulted()};
+    // [expr.prim.lambda.closure] p7 consteval/constexpr are obtained from the call operator
+    const bool          isLambdaStaticInvoker{isCXXMethodDecl and methodDecl->isLambdaStaticInvoker()};
+    const FunctionDecl& constExprDecl{not isLambdaStaticInvoker ? decl
+                                                                : *methodDecl->getParent()->getLambdaCallOperator()};
 
     if(methodDecl) {
         if(requiresComment) {
@@ -3803,9 +3845,22 @@ void CodeGenerator::InsertFunctionNameWithReturnType(const FunctionDecl&       d
         }
     }
 
-    if(decl.isConstexpr()) {
-        if(decl.isConstexprSpecified()) {
-            const bool skipConstexpr{isLambda};
+    if(constExprDecl.isConstexpr()) {
+        const bool skipConstexpr{isLambda and not isa<CXXConversionDecl>(constExprDecl)};
+        // Special treatment for a conversion operator in a captureless lambda. It appears that if the call operator is
+        // consteval the conversion operator must be as well, otherwise it cannot take the address of the invoke
+        // function.
+        const bool isConversionOpWithConstevalCallOp{[&]() {
+            if(methodDecl) {
+                if(const auto callOp = methodDecl->getParent()->getLambdaCallOperator()) {
+                    return callOp->isConsteval();
+                }
+            }
+
+            return false;
+        }()};
+
+        if(not isConversionOpWithConstevalCallOp and constExprDecl.isConstexprSpecified()) {
             if(skipConstexpr) {
                 mOutputFormatHelper.Append(kwCommentStart);
             }
@@ -3816,7 +3871,7 @@ void CodeGenerator::InsertFunctionNameWithReturnType(const FunctionDecl&       d
                 mOutputFormatHelper.Append(kwCCommentEndSpace);
             }
 
-        } else if(decl.isConsteval()) {
+        } else if(isConversionOpWithConstevalCallOp or constExprDecl.isConsteval()) {
             mOutputFormatHelper.Append(kwConstEvalSpace);
         }
     }
@@ -3847,7 +3902,7 @@ void CodeGenerator::InsertFunctionNameWithReturnType(const FunctionDecl&       d
             outputFormatHelper.Append(GetName(decl));
         }
 
-        if(!isLambda && isFirstCxxMethodDecl && decl.isFunctionTemplateSpecialization()) {
+        if(isFirstCxxMethodDecl and decl.isFunctionTemplateSpecialization()) {
             CodeGenerator codeGenerator{outputFormatHelper};
             codeGenerator.InsertTemplateArgs(decl);
         }
@@ -3861,7 +3916,14 @@ void CodeGenerator::InsertFunctionNameWithReturnType(const FunctionDecl&       d
                                                OutputFormatHelper::NameOnly::No,
                                                OutputFormatHelper::GenMissingParamName::Yes);
     } else {
-        outputFormatHelper.AppendParameterList(decl.parameters());
+        // The static invoker needs parameter names to foward parameters to the call operator even when the call
+        // operator doesn't care about them.
+        const OutputFormatHelper::GenMissingParamName genMissingParamName{
+            isLambdaStaticInvoker ? OutputFormatHelper::GenMissingParamName::Yes
+                                  : OutputFormatHelper::GenMissingParamName::No};
+
+        outputFormatHelper.AppendParameterList(
+            decl.parameters(), OutputFormatHelper::NameOnly::No, genMissingParamName);
     }
 
     if(decl.isVariadic()) {

CodeGenerator.h

@@ -256,7 +256,10 @@ class CodeGenerator
 
     void InsertTemplateArg(const TemplateArgumentLoc& arg) { InsertTemplateArg(arg.getArgument()); }
     bool InsertLambdaStaticInvoker(const CXXMethodDecl* cxxMethodDecl);
-    void InsertTemplateParameters(const TemplateParameterList& list);
+
+    STRONG_BOOL(TemplateParamsOnly);  ///! Skip template, type constraints and class/typename.
+    void InsertTemplateParameters(const TemplateParameterList& list,
+                                  const TemplateParamsOnly     templateParamsOnly = TemplateParamsOnly::No);
 
     STRONG_BOOL(InsertInline);
 

OutputFormatHelper.cpp

@@ -46,6 +46,10 @@ void OutputFormatHelper::AppendParameterList(const ArrayRef<ParmVarDecl*> parame
             Append(GetTypeNameAsParameter(type, name));
         } else {
             Append(name);
+
+            if(isa<PackExpansionType>(p->getType())) {
+                Append(kwElipsis);
+            }
         }
     });
 }

docs/Limitations.md

@@ -21,3 +21,29 @@ instantiated and the end of its enclosing namespace. While technically, this inf
 this point.
 
 
+## Lambdas with static invoker
+
+### Captureless lambdas
+
+Issue [#467](https://github.com/andreasfertig/cppinsights/issues/467) raised awareness that captureless lambdas are
+more complicated. According to [expr.prim.lambda.closure] p7, the closure type has a conversion to a function pointer
+function:
+
+> The value returned by this conversion function is the address of a function F that, when invoked, has the same effect as invoking the
+> closure type’s function call operator on a default-constructed instance of the closure type. F is a
+> constexpr function if the function call operator is a constexpr function and is an immediate function if the function call operator is an immediate function.
+
+The code in #467 demonstrates a way to observe that C++ Insights generates less efficient code. Initially, the body of
+the call operator was replicated into the invoke function. However, a captureless lambda with a local `static` variable leads to
+different results. The latest version forwards the call from the invoke function to the call operator. This is still
+less optimal as the compiler does it. Even with `move` and `forward`, we get copies of non-moveable members in a parameter.
+At the same time, the compiler seems to be able to directly forward the invoke call to the call operator.
+
+
+### Lambda captures initialization
+
+C++ Insights shows a constructor for a lambda when it has captures. The compiler does better. It doesn't need a
+constructor, it can direct-initialize the members, and by that, the compiler reduces potential copies as they will happen
+with the C++ Insights version.
+
+

tests/Basic.Start.Main.P5.3Test.expect

@@ -10,15 +10,15 @@ int main()
     }
 
     using retType_3_13 = auto (*)() -> int;
-    inline /*constexpr */ operator retType_3_13 () const noexcept
+    inline constexpr operator retType_3_13 () const noexcept
     {
       return __invoke;
     };
 
     private: 
-    static inline int __invoke()
+    static inline /*constexpr */ int __invoke()
     {
-      return 2;
+      return __lambda_3_13{}.operator()();
     }
 
 

tests/Basic.Start.Main.P5.4Test.expect

@@ -14,15 +14,15 @@ int main(int argc, const char ** argv)
     }
 
     using retType_5_13 = auto (*)() -> int;
-    inline /*constexpr */ operator retType_5_13 () const noexcept
+    inline constexpr operator retType_5_13 () const noexcept
     {
       return __invoke;
     };
 
     private: 
-    static inline int __invoke()
+    static inline /*constexpr */ int __invoke()
     {
-      return 2;
+      return __lambda_5_13{}.operator()();
     }
 
 

tests/ClassOperatorHandler2Test.expect

@@ -104,36 +104,15 @@ int main()
     }
 
     using retType_60_5 = void (*)();
-    inline /*constexpr */ operator retType_60_5 () const noexcept
+    inline constexpr operator retType_60_5 () const noexcept
     {
       return __invoke;
     };
 
     private: 
     static inline void __invoke()
     {
-      int n = 10;
-      f(n);
-      f(42);
-      f(int{});
-      Foo f = Foo(1);
-      f.operator++();
-      f.operator++(0);
-      f.operator--();
-      f.operator--(0);
-      printf("%d\n", f.operator++(0));
-      printf("%d %d\n", f.operator++(0), 1);
-      printf("%d\n", f.operator--(0));
-      printf("%d %d\n", f.operator--(0), 1);
-      Foo * ff = &f;
-      printf("%d\n", ff->Get());
-      ++ff;
-      ff++;
-      ++++ff;
-      --ff;
-      ff--;
-      ----ff;
-      printf("%d\n", f.Get());
+      __lambda_60_5{}.operator()();
     }
 
 

tests/ConstevalTest.expect

@@ -33,15 +33,15 @@ class __lambda_16_15
   }
 
   using retType_16_15 = bool (*)();
-  inline /*constexpr */ operator retType_16_15 () const noexcept
+  inline consteval operator retType_16_15 () const noexcept
   {
     return __invoke;
   };
 
   private: 
-  static inline bool __invoke()
+  static inline consteval bool __invoke()
   {
-    return true;
+    return __lambda_16_15{}.operator()();
   }
 
 

tests/ConstraintAutoParameterTest.expect

@@ -14,15 +14,16 @@ class __lambda_6_15
 
   #ifdef INSIGHTS_USE_TEMPLATE
   template<>
-  inline /*constexpr */ void operator()(int container) const
+  inline /*constexpr */ void operator()<int>(int container) const
   {
   }
   #endif
 
   private: 
   template<C type_parameter_0_0>
-  static inline auto __invoke(type_parameter_0_0 container)
+  static inline /*constexpr */ auto __invoke(type_parameter_0_0 container)
   {
+    return __lambda_6_15{}.operator()<type_parameter_0_0>(container);
   }
 
   public:

tests/EmptyLambda.expect

@@ -9,14 +9,15 @@ void foo()
     }
 
     using retType_2_1 = void (*)();
-    inline /*constexpr */ operator retType_2_1 () const noexcept
+    inline constexpr operator retType_2_1 () const noexcept
     {
       return __invoke;
     };
 
     private: 
-    static inline void __invoke()
+    static inline /*constexpr */ void __invoke()
     {
+      __lambda_2_1{}.operator()();
     }