[Lldb-commits] [PATCH] D158470: [lldb] Add support for recognizing swift mangled names

[Alex Langford via Phabricator via lldb-commits]

bulbazord marked 4 inline comments as done.
bulbazord added inline comments.


================
Comment at: lldb/source/Core/Mangled.cpp:61
 
+  // Swift's older style of mangling used "_T" as a mangling prefix. This can
+  // lead to false positives with other symbols that just so happen to start
----------------
JDevlieghere wrote:
> aprantl wrote:
> > fdeazeve wrote:
> > > JDevlieghere wrote:
> > > > aprantl wrote:
> > > > > Feel free to completely ignore this, it's a pointless micro optimization.
> > > > > 
> > > > > I'm curious if it would be more efficient to write this as 
> > > > > 
> > > > > ```
> > > > > switch (name[0]) {
> > > > >   case '?':     return Mangled::eManglingSchemeMSVC;
> > > > >   case '_': 
> > > > >      switch(name[1]) {
> > > > >         ...
> > > > >      }
> > > > >   case '$': 
> > > > >      switch(name[1]) {
> > > > >         ...
> > > > >      }
> > > > > ```
> > > > > or if it would actually be slower than the chain of "vector" comparisons we have right now?
> > > > I actually started writing a similar comment before discarding it. Even if this code is as hot as I expect it to be, I don't think it would outweigh the complexity and the potential for bugs. I really like how you can glance at the code and see the different mangling schemes and that's the first thing we'd lose. Anyway happy to be proven wrong too. 
> > > Honestly the optimizer is pretty good at doing those, look at the IR: https://godbolt.org/z/PY3TeadbM
> > Sounds good, let's leave it!
> > 
> > Just trolling now: should we use some crazy #ifdef magic to flip the order based on the host platform, such the `?` comes first on windows and `$` comes first on Darwin?
> Did someone say tablegen? 
As much as I love micro-optimizations, let's do all of that in a follow-up. ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158470