That's all correct. I think I got it. There are times where the code is emitting a json::object* that is contained in another json object. Is it good style to return these still as a unique_ptr? I'm looking over what I wrote again, and in some parts I wrap the new json object in a unique_ptr (as a return in some function calls) and in others I use new and delete.
Thanks, Thor Preimesberger On Fri, Sep 27, 2024 at 9:18 AM David Malcolm <dmalc...@redhat.com> wrote: > > On Sat, 2024-09-21 at 22:49 -0500, 4444-thor wrote: > > From: thor <th...@protonmail.com> > > > > This is the second revision of: > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662849.html > > > > I've incorporated the feedback given both by Richard and David - I > > didn't > > find any memory leaks when testing in valgrind :) > > Thanks for the updated patch. > > [...snip...] > > > diff --git a/gcc/tree-emit-json.cc b/gcc/tree-emit-json.cc > > new file mode 100644 > > index 00000000000..df97069b922 > > --- /dev/null > > +++ b/gcc/tree-emit-json.cc > > [...snip...] > > Thanks for using std::unique_ptr, but I may have been unclear in my > earlier email - please use it to indicate ownership of a heap-allocated > pointer... > > > +/* Adds Identifier information to JSON object */ > > + > > +void > > +identifier_node_add_json (tree t, std::unique_ptr<json::object> & json_obj) > > + { > > + const char* buff = IDENTIFIER_POINTER (t); > > + json_obj->set_string("id_to_locale", identifier_to_locale(buff)); > > + buff = IDENTIFIER_POINTER (t); > > + json_obj->set_string("id_point", buff); > > + } > > ...whereas here (and in many other places), the patch has a > > std::unique_ptr<json::object> &json_obj > > (expressing a reference to a a unique_ptr i.e. a non-modifiable non- > null pointer to a pointer that manages the lifetime of a modifiable > json::object) > > where, if I'm reading the code correctly, > > json::object &json_obj > > (expressing a non-modifiable non-null pointer to a modifiable > json::object). > > would be much clearer and simpler. > > [...snip...] > > Hope the above makes sense; sorry if I'm being unclear. > Dave >
