rnk added inline comments.

================
Comment at: clang/lib/CodeGen/CGClass.cpp:3095
+  StringRef CallOpName = CallOpFn->getName();
+  std::string ImplName = ("__impl" + CallOpName).str();
+
----------------
akhuang wrote:
> 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? 
I think we should go back to the last approach you had with the string 
substution, and then we can start a new code review where we consider the two 
alternative approaches. I see lots of examples of `const_cast` in the codebase, 
but maybe the custom name approach is better. Hard to say. Either way, it will 
be easier to resolve the question in a smaller code review. Sorry for the back 
and forth.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154007

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to