This is an archive of the discontinued LLVM Phabricator instance.

[lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation
ClosedPublic

Authored by Michael137 on Feb 6 2023, 7:10 AM.

Details

Summary

Summary

This patch makes the expression evaluator understand
namespace aliases.

This will become important once std::ranges become
more widespread since std::views is defined as:

namespace std {
namespace ranges::views {}

namespace views = ranges::views;
}

Testing

  • Added API test

Diff Detail

Event Timeline

Michael137 created this revision.Feb 6 2023, 7:10 AM
Herald added a project: Restricted Project. · View Herald Transcript
Michael137 requested review of this revision.Feb 6 2023, 7:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 6 2023, 7:10 AM
  • Reword commit message
Michael137 retitled this revision from [WIP][lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation to [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation.Feb 6 2023, 11:41 AM

SGTM.

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
3523

The common pattern in LLVM (and thinking of it, I'm surprised we don't have some kind of helper template for it) is to use find() since operator[] inserts a default-constructed element into the list, which will screw with anyone who concurrently searches using find().

Michael137 updated this revision to Diff 495469.Feb 7 2023, 4:23 AM
  • use find() instead of operator[]
Michael137 updated this revision to Diff 495470.Feb 7 2023, 4:26 AM
  • Reduce scope of variable
aprantl accepted this revision.Feb 9 2023, 1:33 PM
aprantl added inline comments.
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
3523

I think this is a bit too clever for my personal taste :-)

I would go with the namespace-polluting

auto it = m_die_to_decl_ctx.find(die.GetDIE());
if (it != m_die_to_decl_ctx.end())

instead.

This revision is now accepted and ready to land.Feb 9 2023, 1:33 PM
Michael137 updated this revision to Diff 496293.Feb 9 2023, 5:33 PM
  • Minor: simplify condition
Michael137 marked an inline comment as done.Feb 9 2023, 5:33 PM
This revision was landed with ongoing or failed builds.Feb 9 2023, 5:34 PM
This revision was automatically updated to reflect the committed changes.

Broke linux buildbot, investigating...

Michael137 reopened this revision.Feb 12 2023, 11:16 AM

Reopening to Ireland slightly adjusted patch.
New patch makes sure we populate the ManualDWARFIndex with DW_TAG_imported_declaration.

This revision is now accepted and ready to land.Feb 12 2023, 11:16 AM
  • Populate ManualDWARFIndex
  • Remove redundant header
aprantl accepted this revision.Feb 13 2023, 8:51 AM
aprantl added inline comments.
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
358

SGTM.