mib added inline comments.
================ Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h:96-98 + bool IsMangledName(llvm::StringRef name) const override { + return false; + } ---------------- friss wrote: > 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. In D71237, I only added a test for the `function.mangled-name` frame-format entity with a C++ example. 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