[Lldb-commits] [PATCH] D69738: Fix handling for the clang name mangling extension for block invocations

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: lldb/source/Core/Mangled.cpp:106
+
+  return Mangled::eManglingSchemeNone;
+}
----------------
shafik wrote:
> aprantl wrote:
> > You don't need to fix this all at once, but I think it would be even better if this function did something like
> > ```
> > for each language plugin { 
> >   if (mangling_scheme = plugin.isMangledName(...)
> >     ...
> > }
> > ```
> > 
> > I.e., the plugins should be the ones that know about the mangling details.
> There is an old comment akin to that in `IsCPPMangledName`. I did not want to stray too far off into refactoring in this PR.
I think IsCPPMangledName should be called IsMangledName and every language plugin should define one. The fact that you forgot to make the same bugfix in CPlusPlusLanguage::IsCPPMangledName should be motivation enough to not implement the same functionality in two places :-)


================
Comment at: lldb/source/Core/Mangled.cpp:36
+static inline bool cstring_is_mangled(llvm::StringRef s) {
+  return Mangled::GetManglingScheme(s) != Mangled::eManglingSchemeNone;
 }
----------------
I was going to say, the function name doesn't make sense any more, but really we probably don't need this function at all, right?


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

https://reviews.llvm.org/D69738