friss added inline comments.

================
Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h:96-98
+  bool IsMangledName(llvm::StringRef name) const override {
+    return false;
+  }
----------------
shafik wrote:
> xiaobai wrote:
> > friss wrote:
> > > The original code was calling `IsPossibleObjCMethodName` and it looks 
> > > like you completely lose this codepath with this rewrite. Unclear to me 
> > > if it's the right thing to return here, but that's definitely a change in 
> > > behavior.
> > If I understand correctly, Objective-C names aren't mangled in general, and 
> > are usually stored in the `m_demangled` name of `Mangled`, so that code 
> > path should have been bogus to begin with. Maybe I'm missing a detail 
> > though?
> > 
> > I can add a comment to this to clarify that Objective-C names aren't 
> > mangled (if I am correct in my understanding).
> I believe this is true, I have not verified it myself. I believe what happens 
> is that `m_mangled` is just set directly in the Objective-C but it probably 
> would be nice to verify this via a test in `MangledTest.cpp` I actually 
> though @mib had added a test here for Objective-C the other day.
> If I understand correctly, Objective-C names aren't mangled in general, and 
> are usually stored in the m_demangled name of Mangled, so that code path 
> should have been bogus to begin with. Maybe I'm missing a detail though?

I'm just pointing out a change in behavior. If a `Mangled` object had 
`m_mangled` set to an Obj-C method name, `GuessLanguage()` would have returned 
`eLanguageTypeObjC` before your patch. It won't do it after. I don't actually 
know whether we ever put a method name in `m_mangled`, but I bet we do.

Whether Obj-C symbols are actually mangled or not is debatable too. The name of 
the method symbol tells you whether it is a class or instance method and the 
number and names of arguments. Looks pretty close to the information the C++ 
mangling gives you for a symbol.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74187



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

Reply via email to