This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Make frame var --regex always search globals
ClosedPublic

Authored by fdeazeve on Jul 14 2023, 1:07 PM.

Details

Reviewers
jingham
Group Reviewers
Restricted Project
Commits
rG8b19d13fde6e: [lldb] Make frame var --regex always search globals
Summary

Currently frame var --regex sometimes searches globals, sometimes it doesn't.
This happens because StackFrame::GetVariableList always returns the biggest
list it has, regardless of whether only globals were requested or not. In other
words, if a previous call to GetVariableList requested globals, all subsequent
calls will see them.

The implication here is that users of StackFrame::GetVariableList are expected
to filter the results of this function. This is what we do for a vanilla
frame var command. But it is not what we do when --regex is used. This
commit solves the issue by:

  1. Making --regex imply --globals. This matches the behavior of `frame var

<some_name>`, which will also search the global scope.

  1. Making the --regex search respect the command object options.

See the added test for an example of the oddities this patch addresses. Without
the patch, the test fails. However it could be made to pass by calling a plain
frame var before calling frame var --regex A::.

Diff Detail

Event Timeline

fdeazeve created this revision.Jul 14 2023, 1:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 14 2023, 1:07 PM
fdeazeve requested review of this revision.Jul 14 2023, 1:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 14 2023, 1:07 PM
jingham requested changes to this revision.Jul 19 2023, 10:40 AM

LGTM, with a couple suggestions.

You did add a check for requested scope when applying the regex which was missing in the original implementation. That's correct, but I don't think you added a direct test for this addition. Maybe something like stop in main and do:

frame var --regex argc --no-args

that should return no matches with you change, as it should.

lldb/source/Commands/CommandObjectFrame.cpp
502

This is easier to parse if you say "Finds all the variables in all_variables"...

lldb/test/API/commands/frame/var/TestFrameVar.py
61

You can also do this with self.expect, in this case using substr in this case:

self.expect("frame var --regex g_", substr="g_var")

This revision now requires changes to proceed.Jul 19 2023, 10:40 AM
fdeazeve updated this revision to Diff 542122.Jul 19 2023, 11:12 AM

Address review comments

jingham accepted this revision.Jul 19 2023, 11:23 AM

Excellent!

This revision is now accepted and ready to land.Jul 19 2023, 11:23 AM
This revision was automatically updated to reflect the committed changes.