[poppler] Changes to 'better_object'

[Carlos Garcia Campos]

Albert Astals Cid <aacid at kemper.freedesktop.org> writes:

> New branch 'better_object' available with the following commits:
> commit 3c29ded4bee5eadb829ed46af2ec92be57b0077b
> Author: Albert Astals Cid <aacid at kde.org>
> Date:   Fri May 5 23:32:32 2017 +0200
>
>     Make Object free itself on init and destruction
>     
>     Will make for a *much* easier way to code.

Great \o/ I've taken a quick look at it and I have a few questions.

 - Do we really want to have a dead type for Objects after a move? We
   can make moved objects reusable by simply initializing them to
   None. I'm not a C++ expert, but I think the state of a moved object
   is actually undefined, but that doesn't mean you can't make it
   defined in your objects. For example std::unique_ptr has a defined
   behavior, after a move the pointer is initialized to nullptr, like if
   it had just been created and you can reuse a moved unique_ptr by
   using reset().

 - Why keeping copy and shallowCopy? Can't we make copy be the =
   operator and shallowCopy the move?

 - The change in Annot.cc is weird

   Dict *dict = new Dict(xref);
+  dict->decRef();
   dict->add(copyString("Font"), &fontsDictObj);

why do you need to unref an object you have just created?

>     Patches with more std::move coming on top.

Great! I think the other common mistake I've seen in poppler more often
is uninitialized class members. It's easy to forget to initialize
everything in all the constructors, so I think it would also be useful
to move all the initializations to the headers in the class
declarations. So for example

Foo:Foo()
  : ptr(nullptr)
  , n_foo(0)
{
}

becames

class Foo {
private:
  void* ptr { nullptr };
  unsigned n_foo { 0 };
};

What do you think?

>     Most things seem to work though i'm pretty sure some things are broken.
>     
>     NEEDS TESTING
>
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/poppler

-- 
Carlos Garcia Campos
PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/poppler/attachments/20170506/8596fbe5/attachment.sig>