[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

[Amy Huang via Phabricator via cfe-commits]

akhuang added inline comments.


================
Comment at: clang/lib/CodeGen/CGClass.cpp:3095
+  StringRef CallOpName = CallOpFn->getName();
+  std::string ImplName = ("__impl" + CallOpName).str();
+
----------------
rnk wrote:
> akhuang wrote:
> > The old code tried to find the "<lambda_0>" part of the function name and replace the front with "?__impl@", so that the function name was consistent with the mangling scheme.  But apparently there are some cases where when the function name is too long, it uses a hash instead (?), so the attempt to find the <lambda_0> was failing. 
> > 
> > I couldn't find a good way to name this function through the actual mangling code -- is it fine to just add "__impl" to the front and have it not be mangled correctly? 
> Let me suggest one more thing to try here to convince the mangler to do our work for us. You can create a CXXMethodDecl AST node here in CodeGen, and ask the mangler to mangle it for you. If you search CodeGen, you can find other examples of this, notably, for making implicit `this` variable declarations:
> ```
> $ git grep Decl::Create'(' ../clang/lib/CodeGen/
> ../clang/lib/CodeGen/CGBuiltin.cpp:  Args.push_back(ImplicitParamDecl::Create(
> ../clang/lib/CodeGen/CGBuiltin.cpp:    Args.push_back(ImplicitParamDecl::Create(
> ../clang/lib/CodeGen/CGCXXABI.cpp:  auto *ThisDecl = ImplicitParamDecl::Create(
> ```
> 
> Call `CXXMethodDecl::Create`, and copy in all of the attribute from the call operator, but replace the DeclarationNameInfo with an IdentifierInfo of `__impl`.
> 
> If that's not working out, let's land this as is and follow up on fixing the mangling.
I think `CXXMethodDecl::Create` doesn't work because it needs a non-const CXXRecordDecl to construct? 

I added some functions to MicrosoftMangler so that we can pass in a custom DeclarationName, does that seem ok? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154007