rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:819-823
     ArgBackRefMap::iterator Found = TemplateArgBackReferences.find(ND);
     if (Found == TemplateArgBackReferences.end()) {
-      // Mangle full template name into temporary buffer.
-      llvm::SmallString<64> TemplateMangling;
-      llvm::raw_svector_ostream Stream(TemplateMangling);
-      MicrosoftCXXNameMangler Extra(Context, Stream);
-      Extra.mangleTemplateInstantiationName(TD, *TemplateArgs);
-
-      // Use the string backref vector to possibly get a back reference.
-      mangleSourceName(TemplateMangling);
-
-      // Memoize back reference for this type.
-      BackRefVec::iterator StringFound =
-          llvm::find(NameBackReferences, TemplateMangling);
-      if (StringFound != NameBackReferences.end()) {
-        TemplateArgBackReferences[ND] =
-            StringFound - NameBackReferences.begin();
+
+      TemplateArgStringMap::iterator Found = TemplateArgStrings.find(ND);
+      if (Found == TemplateArgStrings.end()) {
----------------
thakis wrote:
> rnk wrote:
> > Here's a thought: could we get by with one map? I think we can be certain 
> > that nothing mangles to integer literals between 0 and 9, since otherwise 
> > the demangler couldn't distinguish those from back references. So, can we 
> > keep the string map, and insert the string "0", "1", etc for back 
> > referenced things?
> I had wondered too. In the end, I figured the int map needs less memory and 
> is almost always enough, so I went with this approach.
I think I'm coming at this more from the perspective of less code complexity, 
fewer ifs, fewer data structures to name and have to understand. We could get 
the memory wins by storing numbers in the map, and then having a side vector of 
numbered strings, indexed by `BackrefId-10`. Then that data structure would be 
seldom used. Anyway, it's all code golf, not really important.


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

https://reviews.llvm.org/D62780



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

Reply via email to