[Lldb-commits] [lldb] r281273 - This is the main part of a change to add breakpoint save and restore to lldb.
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Mon Sep 12 17:29:53 PDT 2016
> On Sep 12, 2016, at 4:57 PM, Zachary Turner <zturner at google.com> wrote:
>
>
>
> On Mon, Sep 12, 2016 at 4:19 PM Jim Ingham via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> Author: jingham
> Date: Mon Sep 12 18:10:56 2016
> New Revision: 281273
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281273&view=rev
> Log:
> This is the main part of a change to add breakpoint save and restore to lldb.
>
> Still to come:
> 1) SB API's
> 2) Testcases
> 3) Loose ends:
> a) serialize Thread options
> b) serialize Exception resolvers
> 4) "break list --file" should list breakpoints contained in a file and
> "break read -f 1 3 5" should then read in only those breakpoints.
>
> <rdar://problem/12611863>
>
> Modified:
> lldb/trunk/include/lldb/Breakpoint/Breakpoint.h
> lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h
> lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h
> lldb/trunk/include/lldb/Breakpoint/BreakpointResolverAddress.h
> lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h
> lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileRegex.h
> lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h
> lldb/trunk/include/lldb/Core/SearchFilter.h
> lldb/trunk/include/lldb/Core/StructuredData.h
> lldb/trunk/include/lldb/Target/LanguageRuntime.h
> lldb/trunk/source/Breakpoint/Breakpoint.cpp
> lldb/trunk/source/Breakpoint/BreakpointOptions.cpp
> lldb/trunk/source/Breakpoint/BreakpointResolver.cpp
> lldb/trunk/source/Breakpoint/BreakpointResolverAddress.cpp
> lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
> lldb/trunk/source/Breakpoint/BreakpointResolverFileRegex.cpp
> lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
> lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
> lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp
> lldb/trunk/source/Core/SearchFilter.cpp
> lldb/trunk/source/Core/StructuredData.cpp
> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
> lldb/trunk/source/Target/LanguageRuntime.cpp
>
> Modified: lldb/trunk/include/lldb/Breakpoint/Breakpoint.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/Breakpoint.h?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Breakpoint/Breakpoint.h (original)
> +++ lldb/trunk/include/lldb/Breakpoint/Breakpoint.h Mon Sep 12 18:10:56 2016
> @@ -27,6 +27,7 @@
> #include "lldb/Core/Event.h"
> #include "lldb/Core/SearchFilter.h"
> #include "lldb/Core/StringList.h"
> +#include "lldb/Core/StructuredData.h"
>
> namespace lldb_private {
>
> @@ -164,6 +165,13 @@ public:
>
> typedef std::shared_ptr<BreakpointPrecondition> BreakpointPreconditionSP;
>
> + // Saving & restoring breakpoints:
> + static lldb::BreakpointSP CreateFromStructuredData(
> + Target &target, StructuredData::ObjectSP &data_object_sp, Error &error);
> +
> + virtual StructuredData::ObjectSP SerializeToStructuredData();
> +
> + static const char *GetSerializationKey() { return "Breakpoint"; }
> //------------------------------------------------------------------
> /// Destructor.
> ///
> @@ -717,7 +725,8 @@ private:
> // to skip certain breakpoint hits. For instance, exception breakpoints
> // use this to limit the stop to certain exception classes, while leaving
> // the condition & callback free for user specification.
> - BreakpointOptions m_options; // Settable breakpoint options
> + std::unique_ptr<BreakpointOptions>
> + m_options_up; // Settable breakpoint options
> BreakpointLocationList
> m_locations; // The list of locations currently found for this breakpoint.
> std::string m_kind_description;
>
> Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h (original)
> +++ lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h Mon Sep 12 18:10:56 2016
> @@ -19,6 +19,7 @@
> // Project includes
> #include "lldb/Core/Baton.h"
> #include "lldb/Core/StringList.h"
> +#include "lldb/Core/StructuredData.h"
> #include "lldb/lldb-private.h"
>
> namespace lldb_private {
> @@ -32,6 +33,52 @@ namespace lldb_private {
>
> class BreakpointOptions {
> public:
> + struct CommandData {
> + CommandData() : user_source(), script_source(), stop_on_error(true) {}
> +
> + ~CommandData() = default;
> +
> + static const char *GetSerializationKey() { return "BKPTCMDData"; }
> +
> + StructuredData::ObjectSP SerializeToStructuredData();
> +
> + static CommandData *
> + CreateFromStructuredData(StructuredData::Dictionary &options_dict,
> + Error &error);
> This should probably return a unique_ptr to prevent memory leaks. And the dictionary should be const.
>
> +
> + StringList user_source;
> + std::string script_source;
> + bool stop_on_error;
> +
> + private:
> + enum OptionNames {
> + UserSource = 0,
> + ScriptSource,
> + StopOnError,
> + LastOptionName
> + };
> +
> + static const char *g_option_names[LastOptionName];
> +
> + static const char *GetKey(enum OptionNames enum_value) {
> + return g_option_names[enum_value];
> + }
> + };
> +
> + class CommandBaton : public Baton {
> + public:
> + CommandBaton(CommandData *data) : Baton(data) {}
> +
> + ~CommandBaton() override {
> + delete ((CommandData *)m_data);
> + m_data = nullptr;
> + }
> +
> + void GetDescription(Stream *s, lldb::DescriptionLevel level) const override;
> + };
> +
> + typedef std::shared_ptr<CommandBaton> CommandBatonSP;
> +
> Seems like it might be useful to have a PtrBaton<T> class that holds a unique_ptr<T>, then you wouldn't need to even make a new class for each type of Baton, they could all just used PtrBaton<T>
I specifically overload the "Add baton" to make a distinction between CommandBatonSP's - which I know about and can serialize - and random pointer holding batons which will most likely be meaningless from run to run. So I don't think I want this.
>
> //------------------------------------------------------------------
> // Constructors and Destructors
> //------------------------------------------------------------------
> @@ -43,35 +90,34 @@ public:
> BreakpointOptions(const BreakpointOptions &rhs);
>
> static BreakpointOptions *CopyOptionsNoCallback(BreakpointOptions &rhs);
> +
> //------------------------------------------------------------------
> - /// This constructor allows you to specify all the breakpoint options.
> + /// This constructor allows you to specify all the breakpoint options
> + /// except the callback. That one is more complicated, and better
> + /// to do by hand.
> ///
> /// @param[in] condition
> /// The expression which if it evaluates to \b true if we are to stop
> ///
> - /// @param[in] callback
> - /// This is the plugin for some code that gets run, returns \b true if we
> - /// are to stop.
> - ///
> - /// @param[in] baton
> - /// Client data that will get passed to the callback.
> - ///
> /// @param[in] enabled
> /// Is this breakpoint enabled.
> ///
> /// @param[in] ignore
> /// How many breakpoint hits we should ignore before stopping.
> ///
> - /// @param[in] thread_id
> - /// Only stop if \a thread_id hits the breakpoint.
> //------------------------------------------------------------------
> - BreakpointOptions(void *condition, BreakpointHitCallback callback,
> - void *baton, bool enabled = true, int32_t ignore = 0,
> - lldb::tid_t thread_id = LLDB_INVALID_THREAD_ID,
> - bool one_shot = false);
> + BreakpointOptions(const char *condition, bool enabled = true,
> + int32_t ignore = 0, bool one_shot = false);
> StringRef condition
>
>
> virtual ~BreakpointOptions();
>
> + static BreakpointOptions *
> + CreateFromStructuredData(StructuredData::Dictionary &data_dict, Error &error);
> unique_ptr<BreakpointOptions>
>
> +
> + virtual StructuredData::ObjectSP SerializeToStructuredData();
> +
> + static const char *GetSerializationKey() { return "BKPTOptions"; }
> StringRef GetSerializationKey
>
> +
> //------------------------------------------------------------------
> // Operators
> //------------------------------------------------------------------
> @@ -131,6 +177,10 @@ public:
> void SetCallback(BreakpointHitCallback callback,
> const lldb::BatonSP &baton_sp, bool synchronous = false);
>
> + void SetCallback(BreakpointHitCallback callback,
> + const BreakpointOptions::CommandBatonSP &command_baton_sp,
> + bool synchronous = false);
> +
> //------------------------------------------------------------------
> /// Remove the callback from this option set.
> //------------------------------------------------------------------
> @@ -279,40 +329,39 @@ public:
>
> //------------------------------------------------------------------
> /// This is the default empty callback.
> - /// @return
> - /// The thread id for which the breakpoint hit will stop,
> - /// LLDB_INVALID_THREAD_ID for all threads.
> //------------------------------------------------------------------
> static bool NullCallback(void *baton, StoppointCallbackContext *context,
> lldb::user_id_t break_id,
> lldb::user_id_t break_loc_id);
>
> - struct CommandData {
> - CommandData() : user_source(), script_source(), stop_on_error(true) {}
> -
> - ~CommandData() = default;
> -
> - StringList user_source;
> - std::string script_source;
> - bool stop_on_error;
> - };
> -
> - class CommandBaton : public Baton {
> - public:
> - CommandBaton(CommandData *data) : Baton(data) {}
> -
> - ~CommandBaton() override {
> - delete ((CommandData *)m_data);
> - m_data = nullptr;
> - }
> -
> - void GetDescription(Stream *s, lldb::DescriptionLevel level) const override;
> - };
> + //------------------------------------------------------------------
> + /// Set a callback based on BreakpointOptions::CommandData.
> + /// @param[in] cmd_data
> + /// A new'ed CommandData object. The breakpoint will take ownership
> + /// of this object.
> + //------------------------------------------------------------------
> + void SetCommandDataCallback(CommandData *cmd_data);
>
> protected:
> //------------------------------------------------------------------
> // Classes that inherit from BreakpointOptions can see and modify these
> //------------------------------------------------------------------
> + enum OptionNames {
> + ConditionText = 0,
> + IgnoreCount,
> + EnabledState,
> + OneShotState,
> + LastOptionName
> + };
> + static const char *g_option_names[LastOptionName];
> +
> + static const char *GetKey(enum OptionNames enum_value) {
> + return g_option_names[enum_value];
> + }
> static StringRef GetKey().
>
> Also this can be an enum class if you use llvm/ADT/BitmaskEnu.h
>
> +
> + static bool BreakpointOptionsCallbackFunction(
> + void *baton, StoppointCallbackContext *context, lldb::user_id_t break_id,
> + lldb::user_id_t break_loc_id);
>
> private:
> //------------------------------------------------------------------
> @@ -320,6 +369,7 @@ private:
> //------------------------------------------------------------------
> BreakpointHitCallback m_callback; // This is the callback function pointer
> lldb::BatonSP m_callback_baton_sp; // This is the client data for the callback
> + bool m_baton_is_command_baton;
> bool m_callback_is_synchronous;
> bool m_enabled;
> bool m_one_shot;
>
> Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h (original)
> +++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h Mon Sep 12 18:10:56 2016
> @@ -136,30 +136,96 @@ public:
> //------------------------------------------------------------------
> virtual void Dump(Stream *s) const = 0;
>
> + /// This section handles serializing and deserializing from StructuredData
> + /// objects.
> +
> + static lldb::BreakpointResolverSP
> + CreateFromStructuredData(StructuredData::Dictionary &resolver_dict,
> + Error &error);
> +
> + virtual StructuredData::ObjectSP SerializeToStructuredData() {
> + return StructuredData::ObjectSP();
> + }
> +
> + static const char *GetSerializationKey() { return "BKPTResolver"; }
> +
> + static const char *GetSerializationSubclassKey() { return "Type"; }
> +
> + static const char *GetSerializationSubclassOptionsKey() { return "Options"; }
> More StringRef opportunities. This makes it very convenient to do comparisons and searching on the return values without resorting to CRT functions.
>
> +
> + StructuredData::DictionarySP
> + WrapOptionsDict(StructuredData::DictionarySP options_dict_sp);
> +
> + //------------------------------------------------------------------
> //------------------------------------------------------------------
> /// An enumeration for keeping track of the concrete subclass that
> /// is actually instantiated. Values of this enumeration are kept in the
> /// BreakpointResolver's SubclassID field. They are used for concrete type
> /// identification.
> enum ResolverTy {
> - FileLineResolver, // This is an instance of BreakpointResolverFileLine
> - AddressResolver, // This is an instance of BreakpointResolverAddress
> - NameResolver, // This is an instance of BreakpointResolverName
> + FileLineResolver = 0, // This is an instance of BreakpointResolverFileLine
> + AddressResolver, // This is an instance of BreakpointResolverAddress
> + NameResolver, // This is an instance of BreakpointResolverName
> FileRegexResolver,
> ExceptionResolver,
> - LastKnownResolverType = ExceptionResolver
> + LastKnownResolverType = ExceptionResolver,
> + UnknownResolver
> };
>
> + // Translate the Ty to name for serialization,
> + // the "+2" is one for size vrs. index, and one for UnknownResolver.
> + static const char *g_ty_to_name[LastKnownResolverType + 2];
> +
> //------------------------------------------------------------------
> /// getResolverID - Return an ID for the concrete type of this object. This
> /// is used to implement the LLVM classof checks. This should not be used
> /// for any other purpose, as the values may change as LLDB evolves.
> unsigned getResolverID() const { return SubclassID; }
>
> + enum ResolverTy GetResolverTy() {
> + if (SubclassID > ResolverTy::LastKnownResolverType)
> + return ResolverTy::UnknownResolver;
> + else
> + return (enum ResolverTy)SubclassID;
> + }
> +
> + const char *GetResolverName() { return ResolverTyToName(GetResolverTy()); }
> +
> + static const char *ResolverTyToName(enum ResolverTy);
> +
> + static ResolverTy NameToResolverTy(const char *name);
> +
> virtual lldb::BreakpointResolverSP
> CopyForBreakpoint(Breakpoint &breakpoint) = 0;
>
> protected:
> + // Used for serializing resolver options:
> + // The options in this enum and the strings in the
> + // g_option_names must be kept in sync.
> + enum OptionNames {
> + AddressOffset = 0,
> + ExactMatch,
> + FileName,
> + Inlines,
> + LanguageName,
> + LineNumber,
> + ModuleName,
> + NameMaskArray,
> + Offset,
> + RegexString,
> + SectionName,
> + SkipPrologue,
> + SymbolNameArray,
> + LastOptionName
> + };
> + static const char *g_option_names[LastOptionName];
> +
> +public:
> + static const char *GetKey(enum OptionNames enum_value) {
> + return g_option_names[enum_value];
> + }
> +
> +protected:
> //------------------------------------------------------------------
> /// SetSCMatchesByLine - Takes a symbol context list of matches which
> /// supposedly represent the same file and
>
> Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverAddress.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolverAddress.h?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Breakpoint/BreakpointResolverAddress.h (original)
> +++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolverAddress.h Mon Sep 12 18:10:56 2016
> @@ -36,6 +36,11 @@ public:
>
> ~BreakpointResolverAddress() override;
>
> + static BreakpointResolver *CreateFromStructuredData(
> + Breakpoint *bkpt, StructuredData::Dictionary &options_dict, Error &error);
> +
> + StructuredData::ObjectSP SerializeToStructuredData() override;
> +
> void ResolveBreakpoint(SearchFilter &filter) override;
>
> void ResolveBreakpointInModules(SearchFilter &filter,
>
> Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h (original)
> +++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h Mon Sep 12 18:10:56 2016
> @@ -33,6 +33,12 @@ public:
> bool check_inlines, bool skip_prologue,
> bool exact_match);
>
> + static BreakpointResolver *
> + CreateFromStructuredData(Breakpoint *bkpt,
> + StructuredData::Dictionary &data_dict, Error &error);
> +
> + StructuredData::ObjectSP SerializeToStructuredData() override;
> +
> ~BreakpointResolverFileLine() override;
>
> Searcher::CallbackReturn SearchCallback(SearchFilter &filter,
>
> Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileRegex.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileRegex.h?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileRegex.h (original)
> +++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileRegex.h Mon Sep 12 18:10:56 2016
> @@ -35,6 +35,11 @@ public:
> Breakpoint *bkpt, RegularExpression ®ex,
> const std::unordered_set<std::string> &func_name_set, bool exact_match);
>
> + static BreakpointResolver *CreateFromStructuredData(
> + Breakpoint *bkpt, StructuredData::Dictionary &options_dict, Error &error);
> +
> + StructuredData::ObjectSP SerializeToStructuredData() override;
> +
> ~BreakpointResolverFileRegex() override;
>
> Searcher::CallbackReturn SearchCallback(SearchFilter &filter,
>
> Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h (original)
> +++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h Mon Sep 12 18:10:56 2016
> @@ -54,9 +54,11 @@ public:
> lldb::LanguageType language, lldb::addr_t offset,
> bool skip_prologue);
>
> - BreakpointResolverName(Breakpoint *bkpt, const char *class_name,
> - const char *method, Breakpoint::MatchType type,
> - lldb::addr_t offset, bool skip_prologue);
> + static BreakpointResolver *
> + CreateFromStructuredData(Breakpoint *bkpt,
> + StructuredData::Dictionary &data_dict, Error &error);
> +
> + StructuredData::ObjectSP SerializeToStructuredData() override;
>
> ~BreakpointResolverName() override;
>
>
> Modified: lldb/trunk/include/lldb/Core/SearchFilter.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/SearchFilter.h?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Core/SearchFilter.h (original)
> +++ lldb/trunk/include/lldb/Core/SearchFilter.h Mon Sep 12 18:10:56 2016
> @@ -15,6 +15,7 @@
> // Other libraries and framework includes
> // Project includes
> #include "lldb/Core/FileSpecList.h"
> +#include "lldb/Core/StructuredData.h"
> #include "lldb/lldb-private.h"
>
> namespace lldb_private {
> @@ -101,6 +102,8 @@ public:
>
> SearchFilter(const SearchFilter &rhs);
>
> + SearchFilter(const lldb::TargetSP &target_sp, unsigned char filterType);
> +
> virtual ~SearchFilter();
>
> SearchFilter &operator=(const SearchFilter &rhs);
> @@ -213,7 +216,60 @@ public:
>
> lldb::SearchFilterSP CopyForBreakpoint(Breakpoint &breakpoint);
>
> + static SearchFilter *
> + CreateFromStructuredData(Target &target,
> + StructuredData::Dictionary &data_dict, Error &error);
> +
> + virtual StructuredData::ObjectSP SerializeToStructuredData() {
> + return StructuredData::ObjectSP();
> + }
> +
> + static const char *GetSerializationKey() { return "SearchFilter"; }
> +
> + static const char *GetSerializationSubclassKey() { return "Type"; }
> +
> + static const char *GetSerializationSubclassOptionsKey() { return "Options"; }
> +
> + enum FilterTy {
> + Unconstrained = 0,
> + Exception,
> + ByModule,
> + ByModules,
> + ByModulesAndCU,
> + LastKnownFilterType = ByModulesAndCU,
> + UnknownFilter
> + };
> +
> + static const char *g_ty_to_name[LastKnownFilterType + 2];
> +
> + enum FilterTy GetFilterTy() {
> + if (SubclassID > FilterTy::LastKnownFilterType)
> + return FilterTy::UnknownFilter;
> + else
> + return (enum FilterTy)SubclassID;
> + }
> +
> + const char *GetFilterName() { return FilterTyToName(GetFilterTy()); }
> +
> + static const char *FilterTyToName(enum FilterTy);
> +
> + static FilterTy NameToFilterTy(const char *name);
> +
> protected:
> + // Serialization of SearchFilter options:
> + enum OptionNames { ModList = 0, CUList, LanguageName, LastOptionName };
> + static const char *g_option_names[LastOptionName];
> +
> + static const char *GetKey(enum OptionNames enum_value) {
> + return g_option_names[enum_value];
> + }
> +
> + StructuredData::DictionarySP
> + WrapOptionsDict(StructuredData::DictionarySP options_dict_sp);
> +
> + void SerializeFileSpecList(StructuredData::DictionarySP &options_dict_sp,
> + OptionNames name, FileSpecList &file_list);
> A reference to a shared_ptr<> seems odd, is this intended?
>
> +
> // These are utility functions to assist with the search iteration. They are
> // used by the
> // default Search method.
> @@ -239,6 +295,8 @@ protected:
> lldb::TargetSP
> m_target_sp; // Every filter has to be associated with a target for
> // now since you need a starting place for the search.
> +private:
> + unsigned char SubclassID;
> };
>
> //----------------------------------------------------------------------
> @@ -250,13 +308,20 @@ protected:
> class SearchFilterForUnconstrainedSearches : public SearchFilter {
> public:
> SearchFilterForUnconstrainedSearches(const lldb::TargetSP &target_sp)
> - : SearchFilter(target_sp) {}
> + : SearchFilter(target_sp, FilterTy::Unconstrained) {}
> +
> ~SearchFilterForUnconstrainedSearches() override = default;
>
> bool ModulePasses(const FileSpec &module_spec) override;
>
> bool ModulePasses(const lldb::ModuleSP &module_sp) override;
>
> + static SearchFilter *
> + CreateFromStructuredData(Target &target,
> + StructuredData::Dictionary &data_dict, Error &error);
> +
> + StructuredData::ObjectSP SerializeToStructuredData() override;
> +
> protected:
> lldb::SearchFilterSP DoCopyForBreakpoint(Breakpoint &breakpoint) override;
> };
> @@ -304,6 +369,12 @@ public:
>
> void Search(Searcher &searcher) override;
>
> + static SearchFilter *
> + CreateFromStructuredData(Target &target,
> + StructuredData::Dictionary &data_dict, Error &error);
> +
> + StructuredData::ObjectSP SerializeToStructuredData() override;
> +
> protected:
> lldb::SearchFilterSP DoCopyForBreakpoint(Breakpoint &breakpoint) override;
>
> @@ -326,6 +397,10 @@ public:
> SearchFilterByModuleList(const lldb::TargetSP &targetSP,
> const FileSpecList &module_list);
>
> + SearchFilterByModuleList(const lldb::TargetSP &targetSP,
> + const FileSpecList &module_list,
> + enum FilterTy filter_ty);
> +
> SearchFilterByModuleList(const SearchFilterByModuleList &rhs);
>
> ~SearchFilterByModuleList() override;
> @@ -350,6 +425,14 @@ public:
>
> void Search(Searcher &searcher) override;
>
> + static SearchFilter *
> + CreateFromStructuredData(Target &target,
> + StructuredData::Dictionary &data_dict, Error &error);
> +
> + StructuredData::ObjectSP SerializeToStructuredData() override;
> +
> + void SerializeUnwrapped(StructuredData::DictionarySP &options_dict_sp);
> +
> protected:
> lldb::SearchFilterSP DoCopyForBreakpoint(Breakpoint &breakpoint) override;
>
> @@ -394,6 +477,12 @@ public:
>
> void Search(Searcher &searcher) override;
>
> + static SearchFilter *
> + CreateFromStructuredData(Target &target,
> + StructuredData::Dictionary &data_dict, Error &error);
> +
> + StructuredData::ObjectSP SerializeToStructuredData() override;
> +
> protected:
> lldb::SearchFilterSP DoCopyForBreakpoint(Breakpoint &breakpoint) override;
>
>
> Modified: lldb/trunk/include/lldb/Core/StructuredData.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/StructuredData.h?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Core/StructuredData.h (original)
> +++ lldb/trunk/include/lldb/Core/StructuredData.h Mon Sep 12 18:10:56 2016
> @@ -390,6 +390,18 @@ public:
> return value_sp;
> }
>
> + bool GetValueForKeyAsBoolean(llvm::StringRef key, bool &result) const {
> + bool success = false;
> + ObjectSP value_sp = GetValueForKey(key);
> + if (value_sp.get()) {
> Not a big deal since this is a short function, but LLVM style is to use early return on failure.
>
> + Boolean *result_ptr = value_sp->GetAsBoolean();
> + if (result_ptr) {
> + result = result_ptr->GetValue();
> + success = true;
> + }
> + }
> + return success;
> + }
> template <class IntType>
> bool GetValueForKeyAsInteger(llvm::StringRef key, IntType &result) const {
> ObjectSP value_sp = GetValueForKey(key);
> @@ -539,6 +551,8 @@ public:
> };
>
> static ObjectSP ParseJSON(std::string json_text);
> +
> + static ObjectSP ParseJSONFromFile(FileSpec &file, Error &error);
> const FileSpec
>
> };
>
> } // namespace lldb_private
>
> Modified: lldb/trunk/include/lldb/Target/LanguageRuntime.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/LanguageRuntime.h?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Target/LanguageRuntime.h (original)
> +++ lldb/trunk/include/lldb/Target/LanguageRuntime.h Mon Sep 12 18:10:56 2016
> @@ -1,5 +1,5 @@
> //===-- LanguageRuntime.h ---------------------------------------------------*-
> -//C++ -*-===//
> +// C++ -*-===//
> //
> // The LLVM Compiler Infrastructure
> //
> @@ -29,6 +29,38 @@
>
> namespace lldb_private {
>
> +class ExceptionSearchFilter : public SearchFilter {
> +public:
> + ExceptionSearchFilter(const lldb::TargetSP &target_sp,
> + lldb::LanguageType language,
> + bool update_module_list = true);
> +
> + ~ExceptionSearchFilter() override = default;
> +
> + bool ModulePasses(const lldb::ModuleSP &module_sp) override;
> Another shared_ptr reference, shouldn't we just pass the shared_ptr by value or the value by reference?
>
> +
> + bool ModulePasses(const FileSpec &spec) override;
> +
> + void Search(Searcher &searcher) override;
> +
> + void GetDescription(Stream *s) override;
> +
> + static SearchFilter *
> + CreateFromStructuredData(Target &target,
> + StructuredData::Dictionary &data_dict, Error &error);
> +
> + StructuredData::ObjectSP SerializeToStructuredData() override;
> +
> +protected:
> + lldb::LanguageType m_language;
> + LanguageRuntime *m_language_runtime;
> + lldb::SearchFilterSP m_filter_sp;
> +
> + lldb::SearchFilterSP DoCopyForBreakpoint(Breakpoint &breakpoint) override;
> +
> + void UpdateModuleListIfNeeded();
> +};
> +
> class LanguageRuntime : public PluginInterface {
> public:
> ~LanguageRuntime() override;
>
> Modified: lldb/trunk/source/Breakpoint/Breakpoint.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/Breakpoint.cpp?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/source/Breakpoint/Breakpoint.cpp (original)
> +++ lldb/trunk/source/Breakpoint/Breakpoint.cpp Mon Sep 12 18:10:56 2016
> @@ -48,16 +48,17 @@ Breakpoint::Breakpoint(Target &target, S
> BreakpointResolverSP &resolver_sp, bool hardware,
> bool resolve_indirect_symbols)
> : m_being_created(true), m_hardware(hardware), m_target(target),
> - m_filter_sp(filter_sp), m_resolver_sp(resolver_sp), m_options(),
> - m_locations(*this), m_resolve_indirect_symbols(resolve_indirect_symbols),
> - m_hit_count(0) {
> + m_filter_sp(filter_sp), m_resolver_sp(resolver_sp),
> + m_options_up(new BreakpointOptions()), m_locations(*this),
> + m_resolve_indirect_symbols(resolve_indirect_symbols), m_hit_count(0) {
> m_being_created = false;
> }
>
> Breakpoint::Breakpoint(Target &new_target, Breakpoint &source_bp)
> : m_being_created(true), m_hardware(source_bp.m_hardware),
> m_target(new_target), m_name_list(source_bp.m_name_list),
> - m_options(source_bp.m_options), m_locations(*this),
> + m_options_up(new BreakpointOptions(*source_bp.m_options_up.get())),
> + m_locations(*this),
> m_resolve_indirect_symbols(source_bp.m_resolve_indirect_symbols),
> m_hit_count(0) {
> // Now go through and copy the filter & resolver:
> @@ -70,6 +71,115 @@ Breakpoint::Breakpoint(Target &new_targe
> //----------------------------------------------------------------------
> Breakpoint::~Breakpoint() = default;
>
> +//----------------------------------------------------------------------
> +// Serialization
> +//----------------------------------------------------------------------
> +StructuredData::ObjectSP Breakpoint::SerializeToStructuredData() {
> + // Serialize the resolver:
> + StructuredData::DictionarySP breakpoint_dict_sp(
> + new StructuredData::Dictionary());
> + StructuredData::DictionarySP breakpoint_contents_sp(
> + new StructuredData::Dictionary());
> +
> + StructuredData::ObjectSP resolver_dict_sp(
> + m_resolver_sp->SerializeToStructuredData());
> + if (!resolver_dict_sp)
> + return StructuredData::ObjectSP();
> +
> + breakpoint_contents_sp->AddItem(BreakpointResolver::GetSerializationKey(),
> + resolver_dict_sp);
> +
> + StructuredData::ObjectSP filter_dict_sp(
> + m_filter_sp->SerializeToStructuredData());
> + if (!filter_dict_sp)
> + return StructuredData::ObjectSP();
> +
> + breakpoint_contents_sp->AddItem(SearchFilter::GetSerializationKey(),
> + filter_dict_sp);
> +
> + StructuredData::ObjectSP options_dict_sp(
> + m_options_up->SerializeToStructuredData());
> + if (!options_dict_sp)
> + return StructuredData::ObjectSP();
> +
> + breakpoint_contents_sp->AddItem(BreakpointOptions::GetSerializationKey(),
> + options_dict_sp);
> +
> + breakpoint_dict_sp->AddItem(GetSerializationKey(), breakpoint_contents_sp);
> + return breakpoint_dict_sp;
> +}
> +
> +lldb::BreakpointSP Breakpoint::CreateFromStructuredData(
> + Target &target, StructuredData::ObjectSP &object_data, Error &error) {
> + BreakpointSP result_sp;
> +
> + StructuredData::Dictionary *breakpoint_dict = object_data->GetAsDictionary();
> +
> + if (!breakpoint_dict || !breakpoint_dict->IsValid()) {
> + error.SetErrorString("Can't deserialize from an invalid data object.");
> + return result_sp;
> + }
> +
> + StructuredData::Dictionary *resolver_dict;
> + bool success = breakpoint_dict->GetValueForKeyAsDictionary(
> + BreakpointResolver::GetSerializationKey(), resolver_dict);
> + if (!success) {
> + error.SetErrorStringWithFormat(
> + "Breakpoint data missing toplevel resolver key");
> + return result_sp;
> + }
> +
> + Error create_error;
> + BreakpointResolverSP resolver_sp =
> + BreakpointResolver::CreateFromStructuredData(*resolver_dict,
> + create_error);
> + if (create_error.Fail()) {
> + error.SetErrorStringWithFormat(
> + "Error creating breakpoint resolver from data: %s.",
> + create_error.AsCString());
> + return result_sp;
> + }
> +
> + StructuredData::Dictionary *filter_dict;
> + success = breakpoint_dict->GetValueForKeyAsDictionary(
> + SearchFilter::GetSerializationKey(), filter_dict);
> + SearchFilterSP filter_sp;
> + if (!success)
> + filter_sp.reset(
> + new SearchFilterForUnconstrainedSearches(target.shared_from_this()));
> std::make_shared is preferable to this pattern. There's a similar pattern for make_unique<>, but it's especially useful for shared_ptr's since it can allocate the object as well as the shared_ptr's control block with a single allocation. As a general principle, it is also more resilient in the face of exceptions in the constructor, although this doesn't matter for us since we disable exceptions.
We use this pattern in a bunch of places. If we want to make a pass and change all of them, that's fine with me. But doing it piecemeal seems less useful.
>
>
> + else {
> + filter_sp.reset(SearchFilter::CreateFromStructuredData(target, *filter_dict,
> + create_error));
> + if (create_error.Fail()) {
> + error.SetErrorStringWithFormat(
> + "Error creating breakpoint filter from data: %s.",
> + create_error.AsCString());
> + return result_sp;
> + }
> + }
> +
> + BreakpointOptions *options = nullptr;
> + StructuredData::Dictionary *options_dict;
> + success = breakpoint_dict->GetValueForKeyAsDictionary(
> + BreakpointOptions::GetSerializationKey(), options_dict);
> + if (success) {
> + options = BreakpointOptions::CreateFromStructuredData(*options_dict,
> + create_error);
> + if (create_error.Fail()) {
> + error.SetErrorStringWithFormat(
> + "Error creating breakpoint options from data: %s.",
> + create_error.AsCString());
> + return result_sp;
> + }
> + }
> + result_sp =
> + target.CreateBreakpoint(filter_sp, resolver_sp, false, false, true);
> + if (result_sp && options) {
> + result_sp->m_options_up.reset(options);
> + }
> + return result_sp;
> +}
> +
> const lldb::TargetSP Breakpoint::GetTargetSP() {
> return m_target.shared_from_this();
> }
> @@ -111,10 +221,10 @@ void Breakpoint::RemoveInvalidLocations(
> // up the individual settings.
>
> void Breakpoint::SetEnabled(bool enable) {
> - if (enable == m_options.IsEnabled())
> + if (enable == m_options_up->IsEnabled())
> return;
>
> - m_options.SetEnabled(enable);
> + m_options_up->SetEnabled(enable);
> if (enable)
> m_locations.ResolveAllBreakpointSites();
> else
> @@ -124,24 +234,24 @@ void Breakpoint::SetEnabled(bool enable)
> : eBreakpointEventTypeDisabled);
> }
>
> -bool Breakpoint::IsEnabled() { return m_options.IsEnabled(); }
> +bool Breakpoint::IsEnabled() { return m_options_up->IsEnabled(); }
> const member function?
>
>
> void Breakpoint::SetIgnoreCount(uint32_t n) {
> - if (m_options.GetIgnoreCount() == n)
> + if (m_options_up->GetIgnoreCount() == n)
> return;
>
> - m_options.SetIgnoreCount(n);
> + m_options_up->SetIgnoreCount(n);
> SendBreakpointChangedEvent(eBreakpointEventTypeIgnoreChanged);
> }
>
> void Breakpoint::DecrementIgnoreCount() {
> - uint32_t ignore = m_options.GetIgnoreCount();
> + uint32_t ignore = m_options_up->GetIgnoreCount();
> if (ignore != 0)
> - m_options.SetIgnoreCount(ignore - 1);
> + m_options_up->SetIgnoreCount(ignore - 1);
> }
>
> uint32_t Breakpoint::GetIgnoreCount() const {
> - return m_options.GetIgnoreCount();
> + return m_options_up->GetIgnoreCount();
> }
>
> bool Breakpoint::IgnoreCountShouldStop() {
> @@ -160,79 +270,81 @@ bool Breakpoint::IgnoreCountShouldStop()
>
> uint32_t Breakpoint::GetHitCount() const { return m_hit_count; }
>
> -bool Breakpoint::IsOneShot() const { return m_options.IsOneShot(); }
> +bool Breakpoint::IsOneShot() const { return m_options_up->IsOneShot(); }
>
> -void Breakpoint::SetOneShot(bool one_shot) { m_options.SetOneShot(one_shot); }
> +void Breakpoint::SetOneShot(bool one_shot) {
> + m_options_up->SetOneShot(one_shot);
> +}
>
> void Breakpoint::SetThreadID(lldb::tid_t thread_id) {
> - if (m_options.GetThreadSpec()->GetTID() == thread_id)
> + if (m_options_up->GetThreadSpec()->GetTID() == thread_id)
> return;
>
> - m_options.GetThreadSpec()->SetTID(thread_id);
> + m_options_up->GetThreadSpec()->SetTID(thread_id);
> SendBreakpointChangedEvent(eBreakpointEventTypeThreadChanged);
> }
>
> lldb::tid_t Breakpoint::GetThreadID() const {
> - if (m_options.GetThreadSpecNoCreate() == nullptr)
> + if (m_options_up->GetThreadSpecNoCreate() == nullptr)
> return LLDB_INVALID_THREAD_ID;
> else
> - return m_options.GetThreadSpecNoCreate()->GetTID();
> + return m_options_up->GetThreadSpecNoCreate()->GetTID();
> }
>
> void Breakpoint::SetThreadIndex(uint32_t index) {
> - if (m_options.GetThreadSpec()->GetIndex() == index)
> + if (m_options_up->GetThreadSpec()->GetIndex() == index)
> return;
>
> - m_options.GetThreadSpec()->SetIndex(index);
> + m_options_up->GetThreadSpec()->SetIndex(index);
> SendBreakpointChangedEvent(eBreakpointEventTypeThreadChanged);
> }
>
> uint32_t Breakpoint::GetThreadIndex() const {
> - if (m_options.GetThreadSpecNoCreate() == nullptr)
> + if (m_options_up->GetThreadSpecNoCreate() == nullptr)
> return 0;
> else
> - return m_options.GetThreadSpecNoCreate()->GetIndex();
> + return m_options_up->GetThreadSpecNoCreate()->GetIndex();
> }
>
> void Breakpoint::SetThreadName(const char *thread_name) {
> - if (m_options.GetThreadSpec()->GetName() != nullptr &&
> - ::strcmp(m_options.GetThreadSpec()->GetName(), thread_name) == 0)
> + if (m_options_up->GetThreadSpec()->GetName() != nullptr &&
> + ::strcmp(m_options_up->GetThreadSpec()->GetName(), thread_name) == 0)
> return;
> Good opportunity to use a StringRef (for both the parameter as well as ThreadSpec::GetName())
>
>
> - m_options.GetThreadSpec()->SetName(thread_name);
> + m_options_up->GetThreadSpec()->SetName(thread_name);
> SendBreakpointChangedEvent(eBreakpointEventTypeThreadChanged);
> }
>
> const char *Breakpoint::GetThreadName() const {
> - if (m_options.GetThreadSpecNoCreate() == nullptr)
> + if (m_options_up->GetThreadSpecNoCreate() == nullptr)
> return nullptr;
> else
> - return m_options.GetThreadSpecNoCreate()->GetName();
> + return m_options_up->GetThreadSpecNoCreate()->GetName();
> }
> StringRef.
>
>
> void Breakpoint::SetQueueName(const char *queue_name) {
> StringRef.
>
> - if (m_options.GetThreadSpec()->GetQueueName() != nullptr &&
> - ::strcmp(m_options.GetThreadSpec()->GetQueueName(), queue_name) == 0)
> + if (m_options_up->GetThreadSpec()->GetQueueName() != nullptr &&
> + ::strcmp(m_options_up->GetThreadSpec()->GetQueueName(), queue_name) == 0)
> return;
>
> - m_options.GetThreadSpec()->SetQueueName(queue_name);
> + m_options_up->GetThreadSpec()->SetQueueName(queue_name);
> SendBreakpointChangedEvent(eBreakpointEventTypeThreadChanged);
> }
>
> const char *Breakpoint::GetQueueName() const {
> StringRef
>
>
> - if (m_options.GetThreadSpecNoCreate() == nullptr)
> + if (m_options_up->GetThreadSpecNoCreate() == nullptr)
> return nullptr;
> else
> - return m_options.GetThreadSpecNoCreate()->GetQueueName();
> + return m_options_up->GetThreadSpecNoCreate()->GetQueueName();
> }
>
> void Breakpoint::SetCondition(const char *condition) {
> StringRef.
>
> - m_options.SetCondition(condition);
> + m_options_up->SetCondition(condition);
> SendBreakpointChangedEvent(eBreakpointEventTypeConditionChanged);
> }
>
> const char *Breakpoint::GetConditionText() const {
> - return m_options.GetConditionText();
> + return m_options_up->GetConditionText();
> }
> StringRef.
>
>
> // This function is used when "baton" doesn't need to be freed
> @@ -240,7 +352,8 @@ void Breakpoint::SetCallback(BreakpointH
> bool is_synchronous) {
> // The default "Baton" class will keep a copy of "baton" and won't free
> // or delete it when it goes goes out of scope.
> - m_options.SetCallback(callback, BatonSP(new Baton(baton)), is_synchronous);
> + m_options_up->SetCallback(callback, BatonSP(new Baton(baton)),
> + is_synchronous);
>
> SendBreakpointChangedEvent(eBreakpointEventTypeCommandChanged);
> }
> @@ -250,17 +363,17 @@ void Breakpoint::SetCallback(BreakpointH
> void Breakpoint::SetCallback(BreakpointHitCallback callback,
> const BatonSP &callback_baton_sp,
> bool is_synchronous) {
> - m_options.SetCallback(callback, callback_baton_sp, is_synchronous);
> + m_options_up->SetCallback(callback, callback_baton_sp, is_synchronous);
> }
>
> -void Breakpoint::ClearCallback() { m_options.ClearCallback(); }
> +void Breakpoint::ClearCallback() { m_options_up->ClearCallback(); }
>
> bool Breakpoint::InvokeCallback(StoppointCallbackContext *context,
> break_id_t bp_loc_id) {
> - return m_options.InvokeCallback(context, GetID(), bp_loc_id);
> + return m_options_up->InvokeCallback(context, GetID(), bp_loc_id);
> }
>
> -BreakpointOptions *Breakpoint::GetOptions() { return &m_options; }
> +BreakpointOptions *Breakpoint::GetOptions() { return m_options_up.get(); }
>
> void Breakpoint::ResolveBreakpoint() {
> if (m_resolver_sp)
>
> Modified: lldb/trunk/source/Breakpoint/BreakpointOptions.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointOptions.cpp?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/source/Breakpoint/BreakpointOptions.cpp (original)
> +++ lldb/trunk/source/Breakpoint/BreakpointOptions.cpp Mon Sep 12 18:10:56 2016
> @@ -17,6 +17,8 @@
> #include "lldb/Core/Stream.h"
> #include "lldb/Core/StringList.h"
> #include "lldb/Core/Value.h"
> +#include "lldb/Interpreter/CommandInterpreter.h"
> +#include "lldb/Interpreter/CommandReturnObject.h"
> #include "lldb/Target/Process.h"
> #include "lldb/Target/Target.h"
> #include "lldb/Target/ThreadSpec.h"
> @@ -24,6 +26,71 @@
> using namespace lldb;
> using namespace lldb_private;
>
> +const char *BreakpointOptions::CommandData::g_option_names
> + [BreakpointOptions::CommandData::OptionNames::LastOptionName]{
> + "UserSource", "ScriptSource", "StopOnError"};
> +
> +StructuredData::ObjectSP
> +BreakpointOptions::CommandData::SerializeToStructuredData() {
> + size_t num_strings = user_source.GetSize();
> + if (num_strings == 0 && script_source.empty()) {
> + // We shouldn't serialize commands if there aren't any, return an empty sp
> + // to indicate this.
> + return StructuredData::ObjectSP();
> + }
> +
> + StructuredData::DictionarySP options_dict_sp(
> + new StructuredData::Dictionary());
> + options_dict_sp->AddBooleanItem(GetKey(OptionNames::StopOnError),
> + stop_on_error);
> +
> + StructuredData::ArraySP user_source_sp(new StructuredData::Array());
> + if (num_strings > 0) {
> Is this necessary? The loop just won't do anything if it's equal to 0.
Thanks. That was part of a previous implementation of not emitting anything when there were no commands.
>
> + for (size_t i = 0; i < num_strings; i++) {
> + StructuredData::StringSP item_sp(
> + new StructuredData::String(user_source[i]));
> + user_source_sp->AddItem(item_sp);
> + options_dict_sp->AddItem(GetKey(OptionNames::UserSource), user_source_sp);
> + }
> + }
> +
> + if (!script_source.empty()) {
> + StructuredData::StringSP item_sp(new StructuredData::String(script_source));
> + options_dict_sp->AddItem(GetKey(OptionNames::ScriptSource), user_source_sp);
> + }
> + return options_dict_sp;
> +}
> +
> +BreakpointOptions::CommandData *
> +BreakpointOptions::CommandData::CreateFromStructuredData(
> + StructuredData::Dictionary &options_dict, Error &error) {
> + std::string script_source;
> + CommandData *data = new CommandData();
> + bool success = options_dict.GetValueForKeyAsBoolean(
> + GetKey(OptionNames::StopOnError), data->stop_on_error);
> +
> + success = options_dict.GetValueForKeyAsString(
> + GetKey(OptionNames::ScriptSource), data->script_source);
> +
> + StructuredData::Array *user_source;
> + success = options_dict.GetValueForKeyAsArray(GetKey(OptionNames::UserSource),
> + user_source);
> + if (success) {
> Early return.
>
> + size_t num_elems = user_source->GetSize();
> + for (size_t i = 0; i < num_elems; i++) {
> + std::string elem_string;
> + success = user_source->GetItemAtIndexAsString(i, elem_string);
> + if (success)
> + data->user_source.AppendString(elem_string);
> + }
> + }
> + return data;
> +}
> +
> +const char *BreakpointOptions::g_option_names
> + [BreakpointOptions::OptionNames::LastOptionName]{
> + "ConditionText", "IgnoreCount", "EnabledState", "OneShotState"};
> +
> bool BreakpointOptions::NullCallback(void *baton,
> StoppointCallbackContext *context,
> lldb::user_id_t break_id,
> @@ -36,15 +103,25 @@ bool BreakpointOptions::NullCallback(voi
> //----------------------------------------------------------------------
> BreakpointOptions::BreakpointOptions()
> : m_callback(BreakpointOptions::NullCallback), m_callback_baton_sp(),
> - m_callback_is_synchronous(false), m_enabled(true), m_one_shot(false),
> - m_ignore_count(0), m_thread_spec_ap(), m_condition_text(),
> - m_condition_text_hash(0) {}
> + m_baton_is_command_baton(false), m_callback_is_synchronous(false),
> + m_enabled(true), m_one_shot(false), m_ignore_count(0), m_thread_spec_ap(),
> + m_condition_text(), m_condition_text_hash(0) {}
> +
> +BreakpointOptions::BreakpointOptions(const char *condition, bool enabled,
> + int32_t ignore, bool one_shot)
> + : m_callback(nullptr), m_baton_is_command_baton(false),
> + m_callback_is_synchronous(false), m_enabled(enabled),
> + m_one_shot(one_shot), m_ignore_count(ignore), m_condition_text(condition),
> + m_condition_text_hash(0)
> +
> +{}
>
> //----------------------------------------------------------------------
> // BreakpointOptions copy constructor
> //----------------------------------------------------------------------
> BreakpointOptions::BreakpointOptions(const BreakpointOptions &rhs)
> : m_callback(rhs.m_callback), m_callback_baton_sp(rhs.m_callback_baton_sp),
> + m_baton_is_command_baton(rhs.m_baton_is_command_baton),
> m_callback_is_synchronous(rhs.m_callback_is_synchronous),
> m_enabled(rhs.m_enabled), m_one_shot(rhs.m_one_shot),
> m_ignore_count(rhs.m_ignore_count), m_thread_spec_ap() {
> @@ -61,6 +138,7 @@ const BreakpointOptions &BreakpointOptio
> operator=(const BreakpointOptions &rhs) {
> m_callback = rhs.m_callback;
> m_callback_baton_sp = rhs.m_callback_baton_sp;
> + m_baton_is_command_baton = rhs.m_baton_is_command_baton;
> m_callback_is_synchronous = rhs.m_callback_is_synchronous;
> m_enabled = rhs.m_enabled;
> m_one_shot = rhs.m_one_shot;
> @@ -91,21 +169,109 @@ BreakpointOptions::CopyOptionsNoCallback
> //----------------------------------------------------------------------
> BreakpointOptions::~BreakpointOptions() = default;
>
> +BreakpointOptions *BreakpointOptions::CreateFromStructuredData(
> + StructuredData::Dictionary &options_dict, Error &error) {
> + bool enabled = true;
> + bool one_shot = false;
> + int32_t ignore_count = 0;
> + std::string condition_text;
> +
> + bool success = options_dict.GetValueForKeyAsBoolean(
> + GetKey(OptionNames::EnabledState), enabled);
> + if (!success) {
> + error.SetErrorStringWithFormat("%s key is not a boolean.",
> + GetKey(OptionNames::EnabledState));
> + return nullptr;
> + }
> +
> + success = options_dict.GetValueForKeyAsBoolean(
> + GetKey(OptionNames::OneShotState), one_shot);
> + if (!success) {
> + error.SetErrorStringWithFormat("%s key is not a boolean.",
> + GetKey(OptionNames::OneShotState));
> + return nullptr;
> + }
> + success = options_dict.GetValueForKeyAsInteger(
> + GetKey(OptionNames::IgnoreCount), ignore_count);
> + if (!success) {
> + error.SetErrorStringWithFormat("%s key is not an integer.",
> + GetKey(OptionNames::IgnoreCount));
> + return nullptr;
> + }
> +
> + BreakpointOptions::CommandData *cmd_data = nullptr;
> + StructuredData::Dictionary *cmds_dict;
> + success = options_dict.GetValueForKeyAsDictionary(
> + CommandData::GetSerializationKey(), cmds_dict);
> + if (success && cmds_dict) {
> + Error cmds_error;
> + cmd_data = CommandData::CreateFromStructuredData(*cmds_dict, cmds_error);
> + if (cmds_error.Fail()) {
> + error.SetErrorStringWithFormat(
> + "Failed to deserialize breakpoint command options: %s.",
> + cmds_error.AsCString());
> + return nullptr;
> + }
> + }
> +
> + BreakpointOptions *bp_options = new BreakpointOptions(
> + condition_text.c_str(), enabled, ignore_count, one_shot);
> + if (cmd_data)
> + bp_options->SetCommandDataCallback(cmd_data);
> + return bp_options;
> +}
> This should return a unique_ptr.
>
> +
> +StructuredData::ObjectSP BreakpointOptions::SerializeToStructuredData() {
> + StructuredData::DictionarySP options_dict_sp(
> + new StructuredData::Dictionary());
> + options_dict_sp->AddBooleanItem(GetKey(OptionNames::EnabledState), m_enabled);
> + options_dict_sp->AddBooleanItem(GetKey(OptionNames::OneShotState),
> + m_one_shot);
> + options_dict_sp->AddIntegerItem(GetKey(OptionNames::IgnoreCount),
> + m_ignore_count);
> + options_dict_sp->AddStringItem(GetKey(OptionNames::ConditionText),
> + m_condition_text);
> + if (m_baton_is_command_baton) {
> + CommandData *cmd_data =
> + static_cast<CommandData *>(m_callback_baton_sp->m_data);
> + StructuredData::ObjectSP commands_sp =
> + cmd_data->SerializeToStructuredData();
> + if (commands_sp) {
> + options_dict_sp->AddItem(
> + BreakpointOptions::CommandData::GetSerializationKey(), commands_sp);
> + }
> + }
> + // FIXME: Need to serialize thread filter...
> + return options_dict_sp;
> +}
> +
> //------------------------------------------------------------------
> // Callbacks
> //------------------------------------------------------------------
> void BreakpointOptions::SetCallback(BreakpointHitCallback callback,
> - const BatonSP &callback_baton_sp,
> + const lldb::BatonSP &callback_baton_sp,
> bool callback_is_synchronous) {
> m_callback_is_synchronous = callback_is_synchronous;
> m_callback = callback;
> m_callback_baton_sp = callback_baton_sp;
> + m_baton_is_command_baton = false;
> +}
> +
> +void BreakpointOptions::SetCallback(
> + BreakpointHitCallback callback,
> + const BreakpointOptions::CommandBatonSP &callback_baton_sp,
> + bool callback_is_synchronous) {
> + m_callback_is_synchronous = callback_is_synchronous;
> + m_callback = callback;
> + m_callback_baton_sp = callback_baton_sp;
> + m_baton_is_command_baton = true;
> }
>
> void BreakpointOptions::ClearCallback() {
> m_callback = BreakpointOptions::NullCallback;
> m_callback_is_synchronous = false;
> m_callback_baton_sp.reset();
> + m_baton_is_command_baton = false;
> }
>
> Baton *BreakpointOptions::GetBaton() { return m_callback_baton_sp.get(); }
> @@ -239,3 +405,49 @@ void BreakpointOptions::CommandBaton::Ge
> s->IndentLess();
> s->IndentLess();
> }
> +
> +void BreakpointOptions::SetCommandDataCallback(CommandData *cmd_data) {
> + CommandBatonSP baton_sp(new CommandBaton(cmd_data));
> + SetCallback(BreakpointOptions::BreakpointOptionsCallbackFunction, baton_sp);
> +}
> +
> +bool BreakpointOptions::BreakpointOptionsCallbackFunction(
> + void *baton, StoppointCallbackContext *context, lldb::user_id_t break_id,
> + lldb::user_id_t break_loc_id) {
> + bool ret_value = true;
> + if (baton == nullptr)
> + return true;
> +
> + CommandData *data = (CommandData *)baton;
> + StringList &commands = data->user_source;
> +
> + if (commands.GetSize() > 0) {
> + ExecutionContext exe_ctx(context->exe_ctx_ref);
> + Target *target = exe_ctx.GetTargetPtr();
> + if (target) {
> + CommandReturnObject result;
> + Debugger &debugger = target->GetDebugger();
> + // Rig up the results secondary output stream to the debugger's, so the
> + // output will come out synchronously
> + // if the debugger is set up that way.
> +
> + StreamSP output_stream(debugger.GetAsyncOutputStream());
> + StreamSP error_stream(debugger.GetAsyncErrorStream());
> + result.SetImmediateOutputStream(output_stream);
> + result.SetImmediateErrorStream(error_stream);
> +
> + CommandInterpreterRunOptions options;
> + options.SetStopOnContinue(true);
> + options.SetStopOnError(data->stop_on_error);
> + options.SetEchoCommands(true);
> + options.SetPrintResults(true);
> + options.SetAddToHistory(false);
> +
> + debugger.GetCommandInterpreter().HandleCommands(commands, &exe_ctx,
> + options, result);
> + result.GetImmediateOutputStream()->Flush();
> + result.GetImmediateErrorStream()->Flush();
> + }
> + }
> + return ret_value;
> +}
>
> Modified: lldb/trunk/source/Breakpoint/BreakpointResolver.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointResolver.cpp?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/source/Breakpoint/BreakpointResolver.cpp (original)
> +++ lldb/trunk/source/Breakpoint/BreakpointResolver.cpp Mon Sep 12 18:10:56 2016
> @@ -15,6 +15,12 @@
> // Project includes
> #include "lldb/Breakpoint/Breakpoint.h"
> #include "lldb/Breakpoint/BreakpointLocation.h"
> +// Have to include the other breakpoint resolver types here so the static create
> +// from StructuredData can call them.
> +#include "lldb/Breakpoint/BreakpointResolverAddress.h"
> +#include "lldb/Breakpoint/BreakpointResolverFileLine.h"
> +#include "lldb/Breakpoint/BreakpointResolverFileRegex.h"
> +#include "lldb/Breakpoint/BreakpointResolverName.h"
> #include "lldb/Core/Address.h"
> #include "lldb/Core/Log.h"
> #include "lldb/Core/ModuleList.h"
> @@ -32,6 +38,32 @@ using namespace lldb;
> //----------------------------------------------------------------------
> // BreakpointResolver:
> //----------------------------------------------------------------------
> +const char *BreakpointResolver::g_ty_to_name[] = {"FileAndLine", "Address",
> + "SymbolName", "SourceRegex",
> + "Exception", "Unknown"};
> +
> +const char *BreakpointResolver::g_option_names
> + [BreakpointResolver::OptionNames::LastOptionName] = {
> + "AddressOffset", "Exact", "FileName", "Inlines", "Language",
> + "LineNumber", "ModuleName", "NameMask", "Offset", "Regex",
> + "SectionName", "SkipPrologue", "SymbolNames"};
> +
> +const char *BreakpointResolver::ResolverTyToName(enum ResolverTy type) {
> + if (type > LastKnownResolverType)
> + return g_ty_to_name[UnknownResolver];
> +
> + return g_ty_to_name[type];
> +}
> +
> +BreakpointResolver::ResolverTy
> +BreakpointResolver::NameToResolverTy(const char *name) {
> + for (size_t i = 0; i < LastKnownResolverType; i++) {
> + if (strcmp(name, g_ty_to_name[i]) == 0)
> Function parameter should be StringRef so we can use operator==.
That seems silly. strcmp is a perfectly good function, and I know this is only ever going to get compared for equality. What would I gain?
>
> + return (ResolverTy)i;
> + }
> + return UnknownResolver;
> +}
> +
> BreakpointResolver::BreakpointResolver(Breakpoint *bkpt,
> const unsigned char resolverTy,
> lldb::addr_t offset)
> @@ -39,6 +71,98 @@ BreakpointResolver::BreakpointResolver(B
>
> BreakpointResolver::~BreakpointResolver() {}
>
> +BreakpointResolverSP BreakpointResolver::CreateFromStructuredData(
> + StructuredData::Dictionary &resolver_dict, Error &error) {
> const dictionary
>
> + BreakpointResolverSP result_sp;
> + if (!resolver_dict.IsValid()) {
> + error.SetErrorString("Can't deserialize from an invalid data object.");
> + return result_sp;
> + }
> +
> + std::string subclass_name;
> +
> + bool success = resolver_dict.GetValueForKeyAsString(
> + GetSerializationSubclassKey(), subclass_name);
> +
> + if (!success) {
> + error.SetErrorStringWithFormat(
> + "Resolver data missing subclass resolver key");
> + return result_sp;
> + }
> +
> + ResolverTy resolver_type = NameToResolverTy(subclass_name.c_str());
> + if (resolver_type == UnknownResolver) {
> + error.SetErrorStringWithFormat("Unknown resolver type: %s.",
> + subclass_name.c_str());
> + return result_sp;
> + }
> +
> + StructuredData::Dictionary *subclass_options = nullptr;
> + success = resolver_dict.GetValueForKeyAsDictionary(
> + GetSerializationSubclassOptionsKey(), subclass_options);
> + if (!success || !subclass_options || !subclass_options->IsValid()) {
> + error.SetErrorString("Resolver data missing subclass options key.");
> + return result_sp;
> + }
> +
> + lldb::addr_t offset;
> + success = subclass_options->GetValueForKeyAsInteger(
> + GetKey(OptionNames::Offset), offset);
> + if (!success) {
> + error.SetErrorString("Resolver data missing offset options key.");
> + return result_sp;
> + }
> +
> + BreakpointResolver *resolver;
> +
> + switch (resolver_type) {
> + case FileLineResolver:
> + resolver = BreakpointResolverFileLine::CreateFromStructuredData(
> + nullptr, *subclass_options, error);
> + break;
> + case AddressResolver:
> + resolver = BreakpointResolverAddress::CreateFromStructuredData(
> + nullptr, *subclass_options, error);
> + break;
> + case NameResolver:
> + resolver = BreakpointResolverName::CreateFromStructuredData(
> + nullptr, *subclass_options, error);
> + break;
> + case FileRegexResolver:
> + resolver = BreakpointResolverFileRegex::CreateFromStructuredData(
> + nullptr, *subclass_options, error);
> + break;
> + case ExceptionResolver:
> + error.SetErrorString("Exception resolvers are hard.");
> + break;
> + default:
> + llvm_unreachable("Should never get an unresolvable resolver type.");
> + }
> +
> + if (!error.Success()) {
> + return result_sp;
> + } else {
> + // Add on the global offset option:
> + resolver->SetOffset(offset);
> + return BreakpointResolverSP(resolver);
> + }
> +}
> +
> +StructuredData::DictionarySP BreakpointResolver::WrapOptionsDict(
> + StructuredData::DictionarySP options_dict_sp) {
> + if (!options_dict_sp || !options_dict_sp->IsValid())
> + return StructuredData::DictionarySP();
> +
> + StructuredData::DictionarySP type_dict_sp(new StructuredData::Dictionary());
> + type_dict_sp->AddStringItem(GetSerializationSubclassKey(), GetResolverName());
> + type_dict_sp->AddItem(GetSerializationSubclassOptionsKey(), options_dict_sp);
> +
> + // Add the m_offset to the dictionary:
> + options_dict_sp->AddIntegerItem(GetKey(OptionNames::Offset), m_offset);
> +
> + return type_dict_sp;
> +}
> +
> void BreakpointResolver::SetBreakpoint(Breakpoint *bkpt) {
> m_breakpoint = bkpt;
> }
>
> Modified: lldb/trunk/source/Breakpoint/BreakpointResolverAddress.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointResolverAddress.cpp?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/source/Breakpoint/BreakpointResolverAddress.cpp (original)
> +++ lldb/trunk/source/Breakpoint/BreakpointResolverAddress.cpp Mon Sep 12 18:10:56 2016
> @@ -17,6 +17,7 @@
> #include "lldb/Breakpoint/BreakpointLocation.h"
> #include "lldb/Core/Log.h"
> #include "lldb/Core/Module.h"
> +#include "lldb/Core/Section.h"
> #include "lldb/Core/StreamString.h"
> #include "lldb/Target/Process.h"
> #include "lldb/Target/Target.h"
> @@ -41,6 +42,62 @@ BreakpointResolverAddress::BreakpointRes
>
> BreakpointResolverAddress::~BreakpointResolverAddress() {}
>
> +BreakpointResolver *BreakpointResolverAddress::CreateFromStructuredData(
> + Breakpoint *bkpt, StructuredData::Dictionary &options_dict, Error &error) {
> + std::string module_name;
> + lldb::addr_t addr_offset;
> + FileSpec module_filespec;
> + bool success;
> +
> + success = options_dict.GetValueForKeyAsInteger(
> + GetKey(OptionNames::AddressOffset), addr_offset);
> + if (!success) {
> + error.SetErrorString("BRFL::CFSD: Couldn't find address offset entry.");
> + return nullptr;
> + }
> + Address address(addr_offset);
> +
> + success = options_dict.HasKey(GetKey(OptionNames::ModuleName));
> + if (success) {
> + success = options_dict.GetValueForKeyAsString(
> + GetKey(OptionNames::ModuleName), module_name);
> + if (!success) {
> + error.SetErrorString("BRA::CFSD: Couldn't read module name entry.");
> + return nullptr;
> + }
> + module_filespec.SetFile(module_name.c_str(), false);
> + }
> + return new BreakpointResolverAddress(bkpt, address, module_filespec);
> +}
> +
> +StructuredData::ObjectSP
> +BreakpointResolverAddress::SerializeToStructuredData() {
> + StructuredData::DictionarySP options_dict_sp(
> + new StructuredData::Dictionary());
> + SectionSP section_sp = m_addr.GetSection();
> + if (section_sp) {
> + ModuleSP module_sp = section_sp->GetModule();
> + ConstString module_name;
> + if (module_sp)
> + module_name.SetCString(module_name.GetCString());
> +
> + options_dict_sp->AddStringItem(GetKey(OptionNames::ModuleName),
> + module_name.GetCString());
> + options_dict_sp->AddIntegerItem(GetKey(OptionNames::AddressOffset),
> + m_addr.GetOffset());
> + } else {
> + options_dict_sp->AddIntegerItem(GetKey(OptionNames::AddressOffset),
> + m_addr.GetOffset());
> + if (m_module_filespec) {
> + options_dict_sp->AddStringItem(GetKey(OptionNames::ModuleName),
> + m_module_filespec.GetPath());
> + }
> + }
> +
> + return WrapOptionsDict(options_dict_sp);
> + return StructuredData::ObjectSP();
> +}
> +
> void BreakpointResolverAddress::ResolveBreakpoint(SearchFilter &filter) {
> // If the address is not section relative, then we should not try to
> // re-resolve it, it is just some
>
> Modified: lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp (original)
> +++ lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp Mon Sep 12 18:10:56 2016
> @@ -36,6 +36,77 @@ BreakpointResolverFileLine::BreakpointRe
>
> BreakpointResolverFileLine::~BreakpointResolverFileLine() {}
>
> +BreakpointResolver *BreakpointResolverFileLine::CreateFromStructuredData(
> + Breakpoint *bkpt, StructuredData::Dictionary &options_dict, Error &error) {
> + std::string filename;
> + uint32_t line_no;
> + bool check_inlines;
> + bool skip_prologue;
> + bool exact_match;
> + bool success;
> +
> + lldb::addr_t offset = 0;
> +
> + success = options_dict.GetValueForKeyAsString(GetKey(OptionNames::FileName),
> + filename);
> + if (!success) {
> + error.SetErrorString("BRFL::CFSD: Couldn't find filename entry.");
> + return nullptr;
> + }
> +
> + success = options_dict.GetValueForKeyAsInteger(
> + GetKey(OptionNames::LineNumber), line_no);
> + if (!success) {
> + error.SetErrorString("BRFL::CFSD: Couldn't find line number entry.");
> + return nullptr;
> + }
> +
> + success = options_dict.GetValueForKeyAsBoolean(GetKey(OptionNames::Inlines),
> + check_inlines);
> + if (!success) {
> + error.SetErrorString("BRFL::CFSD: Couldn't find check inlines entry.");
> + return nullptr;
> + }
> +
> + success = options_dict.GetValueForKeyAsBoolean(
> + GetKey(OptionNames::SkipPrologue), skip_prologue);
> + if (!success) {
> + error.SetErrorString("BRFL::CFSD: Couldn't find skip prologue entry.");
> + return nullptr;
> + }
> +
> + success = options_dict.GetValueForKeyAsBoolean(
> + GetKey(OptionNames::ExactMatch), exact_match);
> + if (!success) {
> + error.SetErrorString("BRFL::CFSD: Couldn't find exact match entry.");
> + return nullptr;
> + }
> +
> + FileSpec file_spec(filename.c_str(), false);
> +
> + return new BreakpointResolverFileLine(bkpt, file_spec, line_no, offset,
> + check_inlines, skip_prologue,
> + exact_match);
> +}
> +
> +StructuredData::ObjectSP
> +BreakpointResolverFileLine::SerializeToStructuredData() {
> + StructuredData::DictionarySP options_dict_sp(
> + new StructuredData::Dictionary());
> +
> + options_dict_sp->AddStringItem(GetKey(OptionNames::FileName),
> + m_file_spec.GetPath());
> + options_dict_sp->AddIntegerItem(GetKey(OptionNames::LineNumber),
> + m_line_number);
> + options_dict_sp->AddBooleanItem(GetKey(OptionNames::Inlines), m_inlines);
> + options_dict_sp->AddBooleanItem(GetKey(OptionNames::SkipPrologue),
> + m_skip_prologue);
> + options_dict_sp->AddBooleanItem(GetKey(OptionNames::ExactMatch),
> + m_exact_match);
> +
> + return WrapOptionsDict(options_dict_sp);
> +}
> +
> Searcher::CallbackReturn
> BreakpointResolverFileLine::SearchCallback(SearchFilter &filter,
> SymbolContext &context,
>
> Modified: lldb/trunk/source/Breakpoint/BreakpointResolverFileRegex.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointResolverFileRegex.cpp?rev=281273&r1=281272&r2=281273&view=diff
> ==============================================================================
> --- lldb/trunk/source/Breakpoint/BreakpointResolverFileRegex.cpp (original)
> +++ lldb/trunk/source/Breakpoint/BreakpointResolverFileRegex.cpp Mon Sep 12 18:10:56 2016
> @@ -36,6 +36,70 @@ BreakpointResolverFileRegex::BreakpointR
>
> BreakpointResolverFileRegex::~BreakpointResolverFileRegex() {}
>
> +BreakpointResolver *BreakpointResolverFileRegex::CreateFromStructuredData(
> + Breakpoint *bkpt, StructuredData::Dictionary &options_dict, Error &error) {
> + bool success;
> +
> + std::string regex_string;
> + success = options_dict.GetValueForKeyAsString(
> + GetKey(OptionNames::RegexString), regex_string);
> + if (!success) {
> + error.SetErrorString("BRFR::CFSD: Couldn't find regex entry.");
> + return nullptr;
> + }
> + RegularExpression regex(regex_string.c_str());
> +
> + bool exact_match;
> + success = options_dict.GetValueForKeyAsBoolean(
> + GetKey(OptionNames::ExactMatch), exact_match);
> + if (!success) {
> + error.SetErrorString("BRFL::CFSD: Couldn't find exact match entry.");
> + return nullptr;
> + }
> +
> + // The names array is optional:
> + std::unordered_set<std::string> names_set;
> + StructuredData::Array *names_array;
> + success = options_dict.GetValueForKeyAsArray(
> + GetKey(OptionNames::SymbolNameArray), names_array);
> + if (success && names_array) {
> + size_t num_names = names_array->GetSize();
> + for (size_t i = 0; i < num_names; i++) {
> + std::string name;
> + success = names_array->GetItemAtIndexAsString(i, name);
> + if (!success) {
> + error.SetErrorStringWithFormat(
> + "BRFR::CFSD: Malformed element %zu in the names array.", i);
> + return nullptr;
> + }
> + names_set.insert(name);
> + }
> + }
> +
> + return new BreakpointResolverFileRegex(bkpt, regex, names_set, exact_match);
> +}
> +
> +StructuredData::ObjectSP
> +BreakpointResolverFileRegex::SerializeToStructuredData() {<
>
>
> Various comments inline.
>
Thanks for the comments.
I don't see the benefit of using StringRef's to return all the key names. I'm generally only ever going to pass them to the StructuredData API's, which makes them into StringRef's transparently on the way in. How would building StringRefs help here?
You also suggested changing a bunch of BreakpointOption API to return StringRef's. OTOH this CL just mechanically changed from m_options to m_options_up, so changing the API was not part of the patches intent. OTOH most of these options (condition, thread name, etc) can take and return NULL strings. I didn't think StringRef's handled null strings well, was I wrong about that? And again, what would I gain by making these StringRef's? I'm just going to stash them away in some storage the object owns, and I'm not going to ever share the storage with the caller. So least common denominator (const char *) seems a better choice here. If the caller wants a StringRef they are welcome to make one.
> Also it would be nice if we could move towards submitting tests at the same time we submit functionality. I'm not a fan of submitting tests at a later date, and I believe it's also against LLVM guidelines.
We are also supposed to do incremental commits. This was getting pretty big already, so it seemed worth getting it in. But I needed to get the whole sketch done and then do the SB API's since I am going to write tests with the SB API's. So I either have to let the patch get bigger before committing it, or commit this part and the tests later. And, we are all adults here, we don't need this much nanny'ing, I think.
Jim
More information about the lldb-commits
mailing list