This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Remove IRExecutionUnit::CollectFallbackNames
ClosedPublic

Authored by bulbazord on Sep 16 2021, 2:32 PM.

Details

Summary

The work that IRExecutionUnit::CollectFallbackNames is basically the
work that CPlusPlusLanguage::GetDemangledFunctionNameWithoutArguments
does already. It's also (at time or writing) specific to C++, so it can
be folded into IRExecutionUnit::CollectCandidateCPlusPlusNames.

Diff Detail

Event Timeline

bulbazord requested review of this revision.Sep 16 2021, 2:32 PM
bulbazord created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptSep 16 2021, 2:32 PM

Looks fine to me. I am assuming the full test suite passes with this change in place?

lldb/source/Expression/IRExecutionUnit.cpp
717

Seems like we should be passing in the "Mangled" object into "FindBestAlternateMangledName" in case it has extra information. For example constructors and destructors have many different types (in charge, not in charge, etc).

Looks fine to me. I am assuming the full test suite passes with this change in place?

It didn't introduce any new test failures on any of my machines.

lldb/source/Expression/IRExecutionUnit.cpp
717

Yes, I agree. I plan on tackling that function next.

I think this looks fine, just verify that this in a C++ case an expression with an extern "C" function call works fine e.g.

extern "C" {
  int g() { return 10;}
}

int main() {
   return g();  // break here and run expr g()
}

I discovered the other day that we mangle extern "C" functions in C++ mode and I have a fix I wanted to run by everyone but I have not had a chance to do it yet.

So this case get resolved in CollectFallbackNames(...).

I think this looks fine, just verify that this in a C++ case an expression with an extern "C" function call works fine e.g.

extern "C" {
  int g() { return 10;}
}

int main() {
   return g();  // break here and run expr g()
}

I discovered the other day that we mangle extern "C" functions in C++ mode and I have a fix I wanted to run by everyone but I have not had a chance to do it yet.

So this case get resolved in CollectFallbackNames(...).

I believe this is already covered by TestExternCSymbols.py in test/API/cpp/extern_c/. CollectFallbackNames did correctly resolve this case, so I made sure this change didn't break that test.

Anything you think I should change or is this good to go?

shafik accepted this revision.Sep 21 2021, 9:11 PM
This revision is now accepted and ready to land.Sep 21 2021, 9:11 PM
This revision was landed with ongoing or failed builds.Sep 22 2021, 11:04 AM
This revision was automatically updated to reflect the committed changes.