This is an archive of the discontinued LLVM Phabricator instance.

[lldb/Interpreter] Make ScriptedProcessInfo more generic
ClosedPublic

Authored by mib on Dec 2 2022, 11:41 PM.

Details

Summary

This patch moves the ScriptedProcessInfo class out of the
ScriptedProcess and hoist it as a standalone interpreter class, so it can be
reused with the Scripted Platform.

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

Diff Detail

Event Timeline

mib created this revision.Dec 2 2022, 11:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2022, 11:41 PM
mib requested review of this revision.Dec 2 2022, 11:41 PM
JDevlieghere requested changes to this revision.Dec 5 2022, 9:35 AM
JDevlieghere added inline comments.
lldb/include/lldb/Utility/ScriptedMetadata.h
12–14 ↗(On Diff #479805)

These are layering violations. Host and Interpreter both depend on Utility. Regardless this class doesn't seem like a good fit for Utility anyway.

This revision now requires changes to proceed.Dec 5 2022, 9:35 AM
mib added inline comments.Dec 5 2022, 10:14 AM
lldb/include/lldb/Utility/ScriptedMetadata.h
12–14 ↗(On Diff #479805)

@JDevlieghere Do you think it'd make more sense to have it in the Interpreter directory ?

mib updated this revision to Diff 480157.Dec 5 2022, 10:26 AM
mib retitled this revision from [lldb/Utility] Make ScriptedProcessInfo more generic to [lldb/Interpreter] Make ScriptedProcessInfo more generic.
mib edited the summary of this revision. (Show Details)

Move class from Utility to Interpreter

bulbazord added inline comments.Dec 5 2022, 10:59 AM
lldb/include/lldb/Interpreter/ScriptedMetadata.h
9–10

This should be LLDB_INTERPRETER_SCRIPTEDMETADATA_H to be consistent with other classes in Interpreter.

mib updated this revision to Diff 480209.Dec 5 2022, 12:50 PM
mib marked an inline comment as done.

Update header guard

JDevlieghere added inline comments.Dec 5 2022, 1:28 PM
lldb/include/lldb/Interpreter/ScriptedMetadata.h
37

Why return a copy? Can this be a StringRef?

mib marked an inline comment as done.Dec 5 2022, 1:50 PM
mib added inline comments.
lldb/include/lldb/Interpreter/ScriptedMetadata.h
37

I guess we can return a StringRef but the member type still needs to be std::string to keep ownership.

mib updated this revision to Diff 480230.Dec 5 2022, 1:55 PM
mib marked an inline comment as done.

Address @JDevlieghere comment

This revision is now accepted and ready to land.Dec 5 2022, 2:09 PM
This revision was automatically updated to reflect the committed changes.