El diumenge, 5 de juny de 2016, a les 10:44:56 CEST, Carlos Garcia Campos va escriure: > Albert Astals Cid <[email protected]> writes: > > El dimecres, 1 de juny de 2016, a les 18:07:44 CEST, Carlos Garcia Campos > > va> > > escriure: > >> Albert Astals Cid <[email protected]> writes: > >> > El dimarts, 31 de maig de 2016, a les 11:46:38 CEST, Carlos Garcia > >> > Campos > >> > va> > >> > > >> > escriure: > >> >> Albert Astals Cid <[email protected]> writes: > >> >> > El dilluns, 30 de maig de 2016, a les 15:43:19 CEST, Carlos Garcia > >> >> > Campos > >> >> > va> > >> >> > > >> >> > escriure: > >> >> >> Albert Astals Cid <[email protected]> writes: > >> >> >> > Hi guys, related to the previous email about std::unique_ptr I > >> >> >> > think > >> >> >> > we > >> >> >> > would greatly benefit of a class that makes Object management > >> >> >> > easier. > >> >> >> > > >> >> >> > Again if you go and check > >> >> >> > https://bugs.freedesktop.org/attachment.cgi? > >> >> >> > id=124163 we're missing lots of free() and it's hard to prove we > >> >> >> > won't > >> >> >> > miss > >> >> >> > more. > >> >> >> > > >> >> >> > My suggestion is adding a class called UniqueObject (better name > >> >> >> > welcome) > >> >> >> > > >> >> >> > which will: > >> >> >> > * Call free on itself when it goes out of scope > >> >> >> > > >> >> >> > So we don't need to add .free() in every other if-chek-error- > > > > return > > > >> >> >> > * Call free on itself when you write on it > >> >> >> > > >> >> >> > So we can do > >> >> >> > > >> >> >> > dict->lookup("Decode", &obj1); > >> >> >> > > >> >> >> > if (obj1.isNull()) { > >> >> >> > > >> >> >> > dict->lookup("D", &obj1); > >> >> >> > > >> >> >> > } > >> >> >> > > >> >> >> > instead of > >> >> >> > > >> >> >> > dict->lookup("Decode", &obj1); > >> >> >> > > >> >> >> > if (obj1.isNull()) { > >> >> >> > > >> >> >> > obj1.free(); > >> >> >> > dict->lookup("D", &obj1); > >> >> >> > > >> >> >> > } > >> >> >> > > >> >> >> > What do you think? > >> >> >> > >> >> >> Why do we need a new class? Wouldn't it be enough to call free in > >> >> >> the > >> >> >> Object destructor and init* methods? > >> >> > > >> >> > Not really enough, we need to fix shallowCopy and the operator= that > >> >> > is > >> >> > used randomly in the code, and also the whole rest of the code. > >> >> > > >> >> > I agree this may be the best, to not have two "Object" systems at > >> >> > the > >> >> > same > >> >> > time, but it's also a very big change that needs removing of all the > >> >> > free() > >> >> > calls (well not really since gree of a free object does nothing but > >> >> > still > >> >> > would look weird). > >> >> > > >> >> > If we wanted to do this i guess we would do: > >> >> > * add destructor with free > >> >> > * call free in the initObj define > >> >> > * Rename shallowCopy to be like "takeObject" that would take the > >> >> > internal > >> >> > > >> >> > data and set the other objcet to none > >> >> > >> >> Or better remove shallowCopy and use std::move() > >> >> > >> >> > * Make operator= private so people are forced to use takeObject > >> >> > >> >> Or make the object non-copyable by defining the copy constructor and > >> >> operator= as =delete. > >> >> > >> >> > * Make free() private so we can easily remove all its uses > >> >> > > >> >> > And maybe that would be all? > >> >> > >> >> It seems so > >> >> > >> >> > Sounds doable-ish? > >> >> > >> >> Definitely. > >> > > >> > I did work a bit on this (your C++11 suggestions) and it's doable, it's > >> > a > >> > bit tricky in Stream where all the Stream seem to share the same Dict > >> > by > >> > virtue of assigning it around, but i think i'd make it work eventually. > >> > >> We could even remove copy() and make = always copy. Dicts are refcounted > >> anyway, so copies should be cheap in any case. But I haven't looked at it > >> in detail yet, just proposed ideas without actually trying anything :-P>> > >> > There's a problem though, doing that we expose C++11 in the "internal" > >> > API, > >> > and I'm not sure the consumers of that internal API will be happy. > >> > > >> > In theory we've said multiple times we don't care, that people should > >> > be > >> > using one of the frontends, but that just doesn't happen, and if we > >> > break > >> > the API in a way say libreoffice or latex can't use us anymore we > >> > basically make upgrading poppler almost impossible for distros. > >> > > >> > So i will now try to do it the way I suggested (that is with some more > >> > manual C++), I suggest we start by limiting C++11 to the .cc files for > >> > now. > >> > > >> > Am I making sense? > >> > >> I would ask LO and pdflatex devs first, maybe it's not a problem at all > >> for them. > > > > So you're volunteering? ;) > > Not really :-P LO is already using C++ 11, at least a subset of it, what > GCC 4.7 supports, which is more than enough for us. For other projects > using our core API, it would be a matter of adding a compile option, and > if we follow LO and we don't require GCC > 4.7, it shouldn't be a big > deal for anybody.
That's LibreOffice, as far as i know there's more users of poppler core. Cheers, Albert _______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
