Author: Pavel Labath Date: 2020-01-09T14:17:17+01:00 New Revision: 5c4661b7784115cb330996b3a6461c5927339aef
URL: https://github.com/llvm/llvm-project/commit/5c4661b7784115cb330996b3a6461c5927339aef DIFF: https://github.com/llvm/llvm-project/commit/5c4661b7784115cb330996b3a6461c5927339aef.diff LOG: [lldb] Modernize OptionValue::SetValueChangedCallback instead of a function pointer + void*, take a std::function. This removes a bunch of repetitive, unsafe void* casts. Added: Modified: lldb/include/lldb/Interpreter/OptionValue.h lldb/include/lldb/Interpreter/OptionValueProperties.h lldb/include/lldb/Interpreter/Property.h lldb/include/lldb/Target/Process.h lldb/include/lldb/Target/Target.h lldb/include/lldb/lldb-private-interfaces.h lldb/source/Interpreter/OptionValueProperties.cpp lldb/source/Interpreter/Property.cpp lldb/source/Target/Process.cpp lldb/source/Target/Target.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Interpreter/OptionValue.h b/lldb/include/lldb/Interpreter/OptionValue.h index 734c92b4bcad..44c7f621a582 100644 --- a/lldb/include/lldb/Interpreter/OptionValue.h +++ b/lldb/include/lldb/Interpreter/OptionValue.h @@ -58,8 +58,7 @@ class OptionValue { eDumpGroupExport = (eDumpOptionCommand | eDumpOptionName | eDumpOptionValue) }; - OptionValue() - : m_callback(nullptr), m_baton(nullptr), m_value_was_set(false) {} + OptionValue() : m_value_was_set(false) {} virtual ~OptionValue() = default; @@ -304,22 +303,19 @@ class OptionValue { m_parent_wp = parent_sp; } - void SetValueChangedCallback(OptionValueChangedCallback callback, - void *baton) { - assert(m_callback == nullptr); - m_callback = callback; - m_baton = baton; + void SetValueChangedCallback(std::function<void()> callback) { + assert(!m_callback); + m_callback = std::move(callback); } void NotifyValueChanged() { if (m_callback) - m_callback(m_baton, this); + m_callback(); } protected: lldb::OptionValueWP m_parent_wp; - OptionValueChangedCallback m_callback; - void *m_baton; + std::function<void()> m_callback; bool m_value_was_set; // This can be used to see if a value has been set // by a call to SetValueFromCString(). It is often // handy to know if an option value was set from the diff --git a/lldb/include/lldb/Interpreter/OptionValueProperties.h b/lldb/include/lldb/Interpreter/OptionValueProperties.h index bea2b3c91e00..980f01183ef5 100644 --- a/lldb/include/lldb/Interpreter/OptionValueProperties.h +++ b/lldb/include/lldb/Interpreter/OptionValueProperties.h @@ -198,8 +198,7 @@ class OptionValueProperties ConstString name); void SetValueChangedCallback(uint32_t property_idx, - OptionValueChangedCallback callback, - void *baton); + std::function<void()> callback); protected: Property *ProtectedGetPropertyAtIndex(uint32_t idx) { diff --git a/lldb/include/lldb/Interpreter/Property.h b/lldb/include/lldb/Interpreter/Property.h index 797aee4be815..76264832705b 100644 --- a/lldb/include/lldb/Interpreter/Property.h +++ b/lldb/include/lldb/Interpreter/Property.h @@ -64,8 +64,7 @@ class Property { uint32_t output_width, bool display_qualified_name) const; - void SetValueChangedCallback(OptionValueChangedCallback callback, - void *baton); + void SetValueChangedCallback(std::function<void()> callback); protected: ConstString m_name; diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 47c5c7870405..2ba996d4995f 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -85,9 +85,6 @@ class ProcessProperties : public Properties { std::chrono::seconds GetUtilityExpressionTimeout() const; protected: - static void OptionValueChangedCallback(void *baton, - OptionValue *option_value); - Process *m_process; // Can be nullptr for global ProcessProperties }; diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 6f8d60731acf..1e9153c401ef 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -209,26 +209,15 @@ class TargetProperties : public Properties { private: // Callbacks for m_launch_info. - static void Arg0ValueChangedCallback(void *target_property_ptr, - OptionValue *); - static void RunArgsValueChangedCallback(void *target_property_ptr, - OptionValue *); - static void EnvVarsValueChangedCallback(void *target_property_ptr, - OptionValue *); - static void InheritEnvValueChangedCallback(void *target_property_ptr, - OptionValue *); - static void InputPathValueChangedCallback(void *target_property_ptr, - OptionValue *); - static void OutputPathValueChangedCallback(void *target_property_ptr, - OptionValue *); - static void ErrorPathValueChangedCallback(void *target_property_ptr, - OptionValue *); - static void DetachOnErrorValueChangedCallback(void *target_property_ptr, - OptionValue *); - static void DisableASLRValueChangedCallback(void *target_property_ptr, - OptionValue *); - static void DisableSTDIOValueChangedCallback(void *target_property_ptr, - OptionValue *); + void Arg0ValueChangedCallback(); + void RunArgsValueChangedCallback(); + void EnvVarsValueChangedCallback(); + void InputPathValueChangedCallback(); + void OutputPathValueChangedCallback(); + void ErrorPathValueChangedCallback(); + void DetachOnErrorValueChangedCallback(); + void DisableASLRValueChangedCallback(); + void DisableSTDIOValueChangedCallback(); // Member variables. ProcessLaunchInfo m_launch_info; diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h index 04b78bcc19f8..27a2c4c3f27f 100644 --- a/lldb/include/lldb/lldb-private-interfaces.h +++ b/lldb/include/lldb/lldb-private-interfaces.h @@ -82,8 +82,6 @@ typedef bool (*BreakpointHitCallback)(void *baton, typedef bool (*WatchpointHitCallback)(void *baton, StoppointCallbackContext *context, lldb::user_id_t watch_id); -typedef void (*OptionValueChangedCallback)(void *baton, - OptionValue *option_value); typedef bool (*ThreadPlanShouldStopHereCallback)( ThreadPlan *current_plan, Flags &flags, lldb::FrameComparison operation, Status &status, void *baton); diff --git a/lldb/source/Interpreter/OptionValueProperties.cpp b/lldb/source/Interpreter/OptionValueProperties.cpp index 4dae930c3a6f..21750cf18615 100644 --- a/lldb/source/Interpreter/OptionValueProperties.cpp +++ b/lldb/source/Interpreter/OptionValueProperties.cpp @@ -60,10 +60,10 @@ void OptionValueProperties::Initialize(const PropertyDefinitions &defs) { } void OptionValueProperties::SetValueChangedCallback( - uint32_t property_idx, OptionValueChangedCallback callback, void *baton) { + uint32_t property_idx, std::function<void()> callback) { Property *property = ProtectedGetPropertyAtIndex(property_idx); if (property) - property->SetValueChangedCallback(callback, baton); + property->SetValueChangedCallback(std::move(callback)); } void OptionValueProperties::AppendProperty(ConstString name, diff --git a/lldb/source/Interpreter/Property.cpp b/lldb/source/Interpreter/Property.cpp index 78209311e2e5..a81098373c25 100644 --- a/lldb/source/Interpreter/Property.cpp +++ b/lldb/source/Interpreter/Property.cpp @@ -292,8 +292,7 @@ void Property::DumpDescription(CommandInterpreter &interpreter, Stream &strm, } } -void Property::SetValueChangedCallback(OptionValueChangedCallback callback, - void *baton) { +void Property::SetValueChangedCallback(std::function<void()> callback) { if (m_value_sp) - m_value_sp->SetValueChangedCallback(callback, baton); + m_value_sp->SetValueChangedCallback(std::move(callback)); } diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index a8fb32dafa89..6711dc37eca6 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -137,19 +137,12 @@ ProcessProperties::ProcessProperties(lldb_private::Process *process) Process::GetGlobalProperties().get()); m_collection_sp->SetValueChangedCallback( ePropertyPythonOSPluginPath, - ProcessProperties::OptionValueChangedCallback, this); + [this] { m_process->LoadOperatingSystemPlugin(true); }); } } ProcessProperties::~ProcessProperties() = default; -void ProcessProperties::OptionValueChangedCallback(void *baton, - OptionValue *option_value) { - ProcessProperties *properties = (ProcessProperties *)baton; - if (properties->m_process) - properties->m_process->LoadOperatingSystemPlugin(true); -} - bool ProcessProperties::GetDisableMemoryCache() const { const uint32_t idx = ePropertyDisableMemCache; return m_collection_sp->GetPropertyAtIndexAsBoolean( diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index e35a10a3f6bf..83e6f3062666 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -3461,29 +3461,24 @@ TargetProperties::TargetProperties(Target *target) // Set callbacks to update launch_info whenever "settins set" updated any // of these properties m_collection_sp->SetValueChangedCallback( - ePropertyArg0, TargetProperties::Arg0ValueChangedCallback, this); + ePropertyArg0, [this] { Arg0ValueChangedCallback(); }); m_collection_sp->SetValueChangedCallback( - ePropertyRunArgs, TargetProperties::RunArgsValueChangedCallback, this); + ePropertyRunArgs, [this] { RunArgsValueChangedCallback(); }); m_collection_sp->SetValueChangedCallback( - ePropertyEnvVars, TargetProperties::EnvVarsValueChangedCallback, this); + ePropertyEnvVars, [this] { EnvVarsValueChangedCallback(); }); m_collection_sp->SetValueChangedCallback( - ePropertyInputPath, TargetProperties::InputPathValueChangedCallback, - this); + ePropertyInputPath, [this] { InputPathValueChangedCallback(); }); m_collection_sp->SetValueChangedCallback( - ePropertyOutputPath, TargetProperties::OutputPathValueChangedCallback, - this); + ePropertyOutputPath, [this] { OutputPathValueChangedCallback(); }); m_collection_sp->SetValueChangedCallback( - ePropertyErrorPath, TargetProperties::ErrorPathValueChangedCallback, - this); + ePropertyErrorPath, [this] { ErrorPathValueChangedCallback(); }); + m_collection_sp->SetValueChangedCallback(ePropertyDetachOnError, [this] { + DetachOnErrorValueChangedCallback(); + }); m_collection_sp->SetValueChangedCallback( - ePropertyDetachOnError, - TargetProperties::DetachOnErrorValueChangedCallback, this); + ePropertyDisableASLR, [this] { DisableASLRValueChangedCallback(); }); m_collection_sp->SetValueChangedCallback( - ePropertyDisableASLR, TargetProperties::DisableASLRValueChangedCallback, - this); - m_collection_sp->SetValueChangedCallback( - ePropertyDisableSTDIO, - TargetProperties::DisableSTDIOValueChangedCallback, this); + ePropertyDisableSTDIO, [this] { DisableSTDIOValueChangedCallback(); }); m_experimental_properties_up.reset(new TargetExperimentalProperties()); m_collection_sp->AppendProperty( @@ -3493,16 +3488,16 @@ TargetProperties::TargetProperties(Target *target) true, m_experimental_properties_up->GetValueProperties()); // Update m_launch_info once it was created - Arg0ValueChangedCallback(this, nullptr); - RunArgsValueChangedCallback(this, nullptr); - // EnvVarsValueChangedCallback(this, nullptr); // FIXME: cause segfault in + Arg0ValueChangedCallback(); + RunArgsValueChangedCallback(); + // EnvVarsValueChangedCallback(); // FIXME: cause segfault in // Target::GetPlatform() - InputPathValueChangedCallback(this, nullptr); - OutputPathValueChangedCallback(this, nullptr); - ErrorPathValueChangedCallback(this, nullptr); - DetachOnErrorValueChangedCallback(this, nullptr); - DisableASLRValueChangedCallback(this, nullptr); - DisableSTDIOValueChangedCallback(this, nullptr); + InputPathValueChangedCallback(); + OutputPathValueChangedCallback(); + ErrorPathValueChangedCallback(); + DetachOnErrorValueChangedCallback(); + DisableASLRValueChangedCallback(); + DisableSTDIOValueChangedCallback(); } else { m_collection_sp = std::make_shared<TargetOptionValueProperties>(ConstString("target")); @@ -3975,81 +3970,54 @@ void TargetProperties::SetRequireHardwareBreakpoints(bool b) { m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b); } -void TargetProperties::Arg0ValueChangedCallback(void *target_property_ptr, - OptionValue *) { - TargetProperties *this_ = - static_cast<TargetProperties *>(target_property_ptr); - this_->m_launch_info.SetArg0(this_->GetArg0()); +void TargetProperties::Arg0ValueChangedCallback() { + m_launch_info.SetArg0(GetArg0()); } -void TargetProperties::RunArgsValueChangedCallback(void *target_property_ptr, - OptionValue *) { - TargetProperties *this_ = - static_cast<TargetProperties *>(target_property_ptr); +void TargetProperties::RunArgsValueChangedCallback() { Args args; - if (this_->GetRunArguments(args)) - this_->m_launch_info.GetArguments() = args; + if (GetRunArguments(args)) + m_launch_info.GetArguments() = args; } -void TargetProperties::EnvVarsValueChangedCallback(void *target_property_ptr, - OptionValue *) { - TargetProperties *this_ = - static_cast<TargetProperties *>(target_property_ptr); - this_->m_launch_info.GetEnvironment() = this_->GetEnvironment(); +void TargetProperties::EnvVarsValueChangedCallback() { + m_launch_info.GetEnvironment() = GetEnvironment(); } -void TargetProperties::InputPathValueChangedCallback(void *target_property_ptr, - OptionValue *) { - TargetProperties *this_ = - static_cast<TargetProperties *>(target_property_ptr); - this_->m_launch_info.AppendOpenFileAction( - STDIN_FILENO, this_->GetStandardInputPath(), true, false); +void TargetProperties::InputPathValueChangedCallback() { + m_launch_info.AppendOpenFileAction(STDIN_FILENO, GetStandardInputPath(), true, + false); } -void TargetProperties::OutputPathValueChangedCallback(void *target_property_ptr, - OptionValue *) { - TargetProperties *this_ = - static_cast<TargetProperties *>(target_property_ptr); - this_->m_launch_info.AppendOpenFileAction( - STDOUT_FILENO, this_->GetStandardOutputPath(), false, true); +void TargetProperties::OutputPathValueChangedCallback() { + m_launch_info.AppendOpenFileAction(STDOUT_FILENO, GetStandardOutputPath(), + false, true); } -void TargetProperties::ErrorPathValueChangedCallback(void *target_property_ptr, - OptionValue *) { - TargetProperties *this_ = - static_cast<TargetProperties *>(target_property_ptr); - this_->m_launch_info.AppendOpenFileAction( - STDERR_FILENO, this_->GetStandardErrorPath(), false, true); +void TargetProperties::ErrorPathValueChangedCallback() { + m_launch_info.AppendOpenFileAction(STDERR_FILENO, GetStandardErrorPath(), + false, true); } -void TargetProperties::DetachOnErrorValueChangedCallback( - void *target_property_ptr, OptionValue *) { - TargetProperties *this_ = - static_cast<TargetProperties *>(target_property_ptr); - if (this_->GetDetachOnError()) - this_->m_launch_info.GetFlags().Set(lldb::eLaunchFlagDetachOnError); +void TargetProperties::DetachOnErrorValueChangedCallback() { + if (GetDetachOnError()) + m_launch_info.GetFlags().Set(lldb::eLaunchFlagDetachOnError); else - this_->m_launch_info.GetFlags().Clear(lldb::eLaunchFlagDetachOnError); + m_launch_info.GetFlags().Clear(lldb::eLaunchFlagDetachOnError); } -void TargetProperties::DisableASLRValueChangedCallback( - void *target_property_ptr, OptionValue *) { - TargetProperties *this_ = - static_cast<TargetProperties *>(target_property_ptr); - if (this_->GetDisableASLR()) - this_->m_launch_info.GetFlags().Set(lldb::eLaunchFlagDisableASLR); +void TargetProperties::DisableASLRValueChangedCallback() { + if (GetDisableASLR()) + m_launch_info.GetFlags().Set(lldb::eLaunchFlagDisableASLR); else - this_->m_launch_info.GetFlags().Clear(lldb::eLaunchFlagDisableASLR); + m_launch_info.GetFlags().Clear(lldb::eLaunchFlagDisableASLR); } -void TargetProperties::DisableSTDIOValueChangedCallback( - void *target_property_ptr, OptionValue *) { - TargetProperties *this_ = - static_cast<TargetProperties *>(target_property_ptr); - if (this_->GetDisableSTDIO()) - this_->m_launch_info.GetFlags().Set(lldb::eLaunchFlagDisableSTDIO); +void TargetProperties::DisableSTDIOValueChangedCallback() { + if (GetDisableSTDIO()) + m_launch_info.GetFlags().Set(lldb::eLaunchFlagDisableSTDIO); else - this_->m_launch_info.GetFlags().Clear(lldb::eLaunchFlagDisableSTDIO); + m_launch_info.GetFlags().Clear(lldb::eLaunchFlagDisableSTDIO); } // Target::TargetEventData _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits