[Lldb-commits] [PATCH] D74187: [lldb] Add method Language::IsMangledName

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk requested changes to this revision.
kwk added a comment.
This revision now requires changes to proceed.

I agree with @friss that there is a change in behavior. I'm not sure it is fixable with what I wrote but I think so.



================
Comment at: lldb/include/lldb/Target/Language.h:191
 
+  virtual bool IsMangledName(llvm::StringRef name) const = 0;
+
----------------
In `Mangled::GuessLanguage()` you call this function with `if (lang->IsMangledName(mangled.GetCString()))`. `mangled` in that case is a `ConstString`. I wonder why you don't pass that to `IsMangledName`? Is a `StringRef` faster? I'm asking because I see  `GetMethodNameVariants` below and that also must be called with a `ConstString`.


================
Comment at: lldb/source/Core/Mangled.cpp:416
-      return lldb::eLanguageTypeC_plus_plus;
-    else if (ObjCLanguage::IsPossibleObjCMethodName(mangled_name))
-      return lldb::eLanguageTypeObjC;
----------------
In the `ObjCLanguage`'s implementation of `IsMangledName()` I hope to see  a call to `sPossibleObjCMethodName`. I think that is what @friss is aiming for with D74187#1865342. 


================
Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h:97
+  bool IsMangledName(llvm::StringRef name) const override {
+    return false;
+  }
----------------
Essentially, just `return ObjCLanguage::IsPossibleObjCMethodName(name)`, no? It is defined only a few lines further down below.


@friss is that what you mean in order to restore the original behavior?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74187