clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land.
A few suggestions to move things over into RenderScriptRuntime.cpp and to possibly use std::unique_ptr, but they are just suggestions. ================ Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:258-259 @@ -263,1 +257,4 @@ + + typedef std::shared_ptr<ScriptDetails> ScriptDetailsSP; + typedef std::shared_ptr<AllocationDetails> AllocationDetailsSP; ---------------- Are you actually handing out shared pointers to anyone? If you are just using std::shared_ptr so you can put things into the vector, you can probably just use std::unique_ptr. ================ Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:298 @@ +297,3 @@ + // then a new Script detail object will be created for this address and returned. + ScriptDetailsSP LookUpScript(lldb::addr_t address, bool create); + ---------------- If you only need memory management you could say that "m_scripts" owns the objects (if they start using std::unique_ptr) and hand out a raw pointer here. Not sure on your memory management needs though. ================ Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:304 @@ +303,3 @@ + // detail object will be created for this address and returned. + AllocationDetailsSP LookUpAllocation (lldb::addr_t address, bool create); +}; ---------------- Ditto above comment. ================ Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:307-417 @@ -295,2 +306,113 @@ + +// The empirical_type adds a basic level of validation to arbitrary data +// allowing us to track if data has been discovered and stored or not. +// An empirical_type will be marked as valid only if it has been explicitly assigned to. +template <typename type_t> +class empirical_type +{ + public: + // Ctor. Contents is invalid when constructed. + empirical_type() + : valid(false) + {} + + // Return true and copy contents to out if valid, else return false. + bool get(type_t& out) const + { + if (valid) + out = data; + return valid; + } + + // Return a pointer to the contents or nullptr if it was not valid. + const type_t* get() const + { + return valid ? &data : nullptr; + } + + // Assign data explicitly. + void set(const type_t in) + { + data = in; + valid = true; + } + + // Mark contents as invalid. + void invalidate() + { + valid = false; + } + + // Returns true if this type contains valid data. + bool isValid() const + { + return valid; + } + + // Assignment operator. + empirical_type<type_t>& operator = (const type_t in) + { + set(in); + return *this; + } + + // Dereference operator returns contents. + // Warning: Will assert if not valid so use only when you know data is valid. + const type_t& operator * () const + { + assert(valid); + return data; + } + + protected: + bool valid; + type_t data; +}; + +// The ScriptDetails class collects data associated with a single script instance. +struct RenderScriptRuntime::ScriptDetails +{ + enum ScriptType + { + eScript, + eScriptC + }; + + // The derived type of the script. + empirical_type<ScriptType> type; + // The name of the original source file. + empirical_type<std::string> resName; + // Path to script .so file on the device. + empirical_type<std::string> scriptDyLib; + // Directory where kernel objects are cached on device. + empirical_type<std::string> cacheDir; + // Pointer to the context which owns this script. + empirical_type<lldb::addr_t> context; + // Pointer to the script object itself. + empirical_type<lldb::addr_t> script; +}; + +// This AllocationDetails class collects data associated with a single +// allocation instance. +struct RenderScriptRuntime::AllocationDetails +{ + enum DataType + { + eInt, + }; + + enum Dimension + { + e1d, + e2d, + e3d, + eCubeMap, + }; + + empirical_type<DataType> type; + empirical_type<Dimension> dimension; + empirical_type<lldb::addr_t> address; + empirical_type<lldb::addr_t> dataPtr; + empirical_type<lldb::addr_t> context; }; ---------------- Does anyone really need to see these objects outside of RenderScriptRuntime.cpp? If not, move these into RenderScriptRuntime.cpp. Repository: rL LLVM http://reviews.llvm.org/D12936 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits