[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

Mon Aug 23 18:32:18 PDT 2021

jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

I like the change in general, anywhere that we're explicitly listing languages in generic code we're probably doing it wrong...  So far as I can tell you've reconstructed the old behaviors correctly in the new setup.  I left one trivial comment, but really this LGTM.

================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:73
+  llvm::StringRef basename = method.GetBasename();
+  if (basename.empty()) {
+    if (CPlusPlusLanguage::ExtractContextAndIdentifier(
----------------
The old code checked method.IsValid before pulling out the base name.  It seems sensible that an invalid MethodName would return an empty base name, but that does seem to rely a little on implicit behavior?  Not sure that's a big deal.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108229