From 8a16fa6a82587eb338b16c5768a9c3dd20e24443 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Wed, 23 Mar 2022 12:05:47 +0100 Subject: Implement C++ methods for DNA structures This change makes it possible to add implementation of common C++ methods for DNA structures which helps ensuring unsafe operations like shallow copy are done explicitly. For example, creating a shallow copy used to be: Object temp_object = *input_object; In the C++ context it was seen like the temp_object is properly decoupled from the input object, while in the reality is it not. Now this code becomes: Object temp_object = blender::dna::shallow_copy(*input_object); The copy and move constructor and assignment operators are now explicitly disabled. Other than a more explicit resource management this change also solves a lot of warnings generated by the implicitly defined copy constructors w.r.t dealing with deprecated fields. These warnings were generated by Apple Clang when a shallow object copy was created via implicitly defined copy constructor. In order to enable C++ methods for DNA structures a newly added macro `DNA_DEFINE_CXX_METHODS()` is to be used: tpyedef struct Object { DNA_DEFINE_CXX_METHODS(Object) ... } Object; For the shallow copy use `blender::dna::shallow_copy()`. The implementation of the memcpy is hidden via an internal DNA function to avoid pulling `string.h` into every DNA header. This means that the solution does not affect on the headers dependencies. --- Ideally `DNA_shallow_copy` would be defined in a more explicit header, but don;t think we have a suitable one already. Maybe we can introduce `DNA_access.h` ? Differential Revision: https://developer.blender.org/D14427 --- source/blender/makesdna/DNA_defs.h | 81 ++++++++++++++++++++++++++++++ source/blender/makesdna/DNA_object_types.h | 2 + source/blender/makesdna/intern/dna_utils.c | 12 +++++ source/blender/makesdna/intern/makesdna.c | 12 +++++ 4 files changed, 107 insertions(+) (limited to 'source/blender/makesdna') diff --git a/source/blender/makesdna/DNA_defs.h b/source/blender/makesdna/DNA_defs.h index b4230209dd5..283eacf1a16 100644 --- a/source/blender/makesdna/DNA_defs.h +++ b/source/blender/makesdna/DNA_defs.h @@ -46,3 +46,84 @@ /* non-id name variables should use this length */ #define MAX_NAME 64 + +/* #DNA_DEFINE_CXX_METHODS is used to define C++ methods which are needed for proper/safe resource + * management, making unsafe (from an ownership perspective: i.e. pointers which sometimes needs to + * be set to nullptr on copy, sometimes needs to be dupalloc-ed) operations explicit, and taking + * care of compiler specific warnings when dealing with members marked with DNA_DEPRECATED. + * + * The `class_name` argument is to match the structure name the macro is used from. + * + * Typical usage example: + * + * typedef struct Object { + * DNA_DEFINE_CXX_METHODS(Object) + * } Object; + */ +#ifndef __cplusplus +# define DNA_DEFINE_CXX_METHODS(class_name) +#else + +/* Forward-declared here since there is no simple header file to be pulled for this functionality. + * Avoids pulling `string.h` from this header to get access to #memcpy. */ +extern "C" void _DNA_internal_memcpy(void *dst, const void *src, size_t size); + +namespace blender::dna::internal { + +template class ShallowDataConstRef { + public: + constexpr explicit ShallowDataConstRef(const T &ref) : ref_(ref) + { + } + + inline const T *get_pointer() const + { + return &ref_; + } + + private: + const T &ref_; +}; + +} // namespace blender::dna::internal + +# define DNA_DEFINE_CXX_METHODS(class_name) \ + class_name() = default; \ + ~class_name() = default; \ + /* Delete copy and assignment, which are not safe for resource ownership. */ \ + class_name(const class_name &other) = delete; \ + class_name(class_name &&other) noexcept = delete; \ + class_name &operator=(const class_name &other) = delete; \ + class_name &operator=(class_name &&other) = delete; \ + /* Support for shallow copy. */ \ + class_name(const blender::dna::internal::ShallowDataConstRef ref) \ + { \ + _DNA_internal_memcpy(this, ref.get_pointer(), sizeof(class_name)); \ + } \ + class_name &operator=(const blender::dna::internal::ShallowDataConstRef ref) \ + { \ + if (this != ref.get_pointer()) { \ + _DNA_internal_memcpy(this, ref.get_pointer(), sizeof(class_name)); \ + } \ + return *this; \ + } + +namespace blender::dna { + +/* Creates shallow copy of the given object. + * The entire object is copied as-is using memory copy. + * + * Typical usage: + * Object temp_object = blender::dna::shallow_copy(*input_object); + * + * From the implementation detail go via copy constructor/assign operator defined in the structure. + */ +template +[[nodiscard]] inline internal::ShallowDataConstRef shallow_copy(const T &other) +{ + return internal::ShallowDataConstRef(other); +} + +} // namespace blender::dna + +#endif diff --git a/source/blender/makesdna/DNA_object_types.h b/source/blender/makesdna/DNA_object_types.h index 9e0bf7dcc5a..c3708e25ee7 100644 --- a/source/blender/makesdna/DNA_object_types.h +++ b/source/blender/makesdna/DNA_object_types.h @@ -233,6 +233,8 @@ enum eObjectLineArt_Flags { }; typedef struct Object { + DNA_DEFINE_CXX_METHODS(Object) + ID id; /** Animation data (must be immediately after id for utilities to use it). */ struct AnimData *adt; diff --git a/source/blender/makesdna/intern/dna_utils.c b/source/blender/makesdna/intern/dna_utils.c index 8c86ef69ebd..560b91b925e 100644 --- a/source/blender/makesdna/intern/dna_utils.c +++ b/source/blender/makesdna/intern/dna_utils.c @@ -308,3 +308,15 @@ const char *DNA_struct_rename_legacy_hack_alias_from_static(const char *name) } /** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Internal helpers for C++ + * \{ */ + +void _DNA_internal_memcpy(void *dst, const void *src, size_t size); +void _DNA_internal_memcpy(void *dst, const void *src, const size_t size) +{ + memcpy(dst, src, size); +} + +/** \} */ diff --git a/source/blender/makesdna/intern/makesdna.c b/source/blender/makesdna/intern/makesdna.c index 0d2f265a9b5..12ec7262906 100644 --- a/source/blender/makesdna/intern/makesdna.c +++ b/source/blender/makesdna/intern/makesdna.c @@ -620,6 +620,7 @@ static int preprocess_include(char *maindata, const int maindata_len) int newlen = 0; comment = 0; a = maindata_len; + bool skip_until_closing_brace = false; while (a--) { if (cp[0] == '/' && cp[1] == '*') { @@ -646,6 +647,17 @@ static int preprocess_include(char *maindata, const int maindata_len) a -= 13; cp += 13; } + else if (match_identifier(cp, "DNA_DEFINE_CXX_METHODS")) { + /* single values are skipped already, so decrement 1 less */ + a -= 21; + cp += 21; + skip_until_closing_brace = true; + } + else if (skip_until_closing_brace) { + if (cp[0] == ')') { + skip_until_closing_brace = false; + } + } else { md[0] = cp[0]; md++; -- cgit v1.2.3