rsmith added inline comments.

================
Comment at: lib/AST/ItaniumMangle.cpp:668
@@ -664,3 +667,3 @@
   llvm::raw_svector_ostream FunctionEncodingStream(FunctionEncodingBuf);
   CXXNameMangler FunctionEncodingMangler(*this, FunctionEncodingStream);
   // Output name of the function.
----------------
DmitryPolukhin wrote:
> rsmith wrote:
> > Maybe it'd be simpler to just override the output stream here rather than 
> > creating a new mangler?
> I'm not sure that it is simpler because it will also require substitutions 
> save/restore for name mangling (mangleNameWithAbiTags) to produce the same 
> substitutions twice (without implicate abi_tags and later with implicit 
> abi_tags). IMHO, it is almost identical approaches but current one is a bit 
> cleaner because it copies explicitly everything required from temp to outer 
> mangler.
I think we'd want to override the output stream to write to a temporary buffer 
at the point when we would otherwise write out the ABI tags, to avoid needing 
to save/restore any substitutions. But I agree, that seems more invasive than 
what you're doing here. I'll leave this up to you.

================
Comment at: lib/AST/ItaniumMangle.cpp:4439
@@ +4438,3 @@
+  if (Other.SeqID > SeqID) {
+    Substitutions = Other.Substitutions;
+    SeqID = Other.SeqID;
----------------
Please avoid the cost of a full copy here by making this a destructive 
operation on `Other` -- use `swap` or move-assignment here. (We'll still be 
paying for one copy of the substitution set per function mangling, which is 
unfortunate / undesirable.)


https://reviews.llvm.org/D24704



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

Reply via email to