Page MenuHomePhabricator

[lldb] Move ClangModulesDeclVendor ownership to ClangPersistentVariables from Target
ClosedPublic

Authored by bulbazord on May 19 2021, 2:04 PM.

Details

Summary

More decoupling of plugins and non-plugins. Target doesn't need to
manage ClangModulesDeclVendor and ClangPersistentVariables is always available
in situations where you need ClangModulesDeclVendor.

Diff Detail

Event Timeline

bulbazord created this revision.May 19 2021, 2:04 PM
bulbazord requested review of this revision.May 19 2021, 2:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2021, 2:04 PM

First and foremost: Cool new username. +1 from me.

Otherwise I think Shafik's comment in D79752 still applies here. Could you make a short utility function for that chunk of code and put in ClangASTSource (ClangExpressionDeclMap inherits from this, so you can remove that part form both classes):

auto persistent_vars = llvm::cast<ClangPersistentVariables>(
    m_target->GetPersistentExpressionStateForLanguage(lldb::eLanguageTypeC));
std::shared_ptr<ClangModulesDeclVendor> modules_decl_vendor =
    persistent_vars->GetClangModulesDeclVendor();

Just something like ClangModulesDeclVendor &ClangASTSource::GetClangModulesDeclVendor() { ... } seems fine.

Beside that I think this is good to go.

teemperor requested changes to this revision.May 19 2021, 2:14 PM
This revision now requires changes to proceed.May 19 2021, 2:14 PM

First and foremost: Cool new username. +1 from me.

+2

bulbazord updated this revision to Diff 346841.May 20 2021, 1:27 PM

Add helper in ClangASTSource

teemperor accepted this revision.May 20 2021, 1:44 PM

LGTM. Soon LLDB will be as nicely layered (and maybe as tasty) as some good Schichttorte

This revision is now accepted and ready to land.May 20 2021, 1:44 PM
This revision was landed with ongoing or failed builds.May 24 2021, 1:48 PM
This revision was automatically updated to reflect the committed changes.