Commit dd4f30226f3d4738e198dfcb944ce4cdc7e50a86

Authored by Jay Berkenbilt
1 parent df2f5c6a

Rework PointerHolder transition to make it smoother

* Don't surprise people with deprecation warnings
* Provide detailed instructions and support for the transition
ChangeLog
... ... @@ -47,11 +47,6 @@
47 47 includes so you don't have to try to include a header that won't
48 48 be there.
49 49  
50   - * PointerHolder: deprecate getPointer() and getRefcount(). If you
51   - don't want to disable deprecation warnings in general but are not
52   - ready to tackle this change yet, you can define
53   - NO_POINTERHOLDER_DEPRECATION to suppress those.
54   -
55 50 * PointerHolder: add a get() method and a use_count() method for
56 51 compatibility with std::shared_ptr. In qpdf 11, qpdf's APIs will
57 52 switch to using std::shared_ptr instead of PointerHolder, though
... ...
... ... @@ -388,35 +388,65 @@ Other notes:
388 388 PointerHolder to std::shared_ptr
389 389 ================================
390 390  
391   -Remember to update the smart-pointers section of the manual in
392   -design.rst.
  391 +To perform update:
393 392  
394   -Once all deprecation warnings are cleared up (changing getPointer() to
395   -get() and getRefcount() to use_count()), the only real issues are that
396   -implicit assignment of a pointer to a shared_ptr doesn't work while it
397   -does for PointerHolder and containers are different. Using auto takes
398   -care of containers.
  393 +Cherry-pick pointerholder branch commit
399 394  
400   -PointerHolder<X> x = new X() -->
401   -auto x = std::make_shared<X>()
  395 +Upgrade just the library. This is not necessary, but it's an added
  396 +check that the compatibility code works since it will show that tests,
  397 +examples, and CLI will work properly with the upgraded APIs, which
  398 +provides some assurance that other people will have a smooth time with
  399 +their code.
402 400  
403   -PointerHolder<Base> x = new Derived() -->
404   -auto x = std::shared_ptr<Base>(new Derived())
  401 +patrepl s/PointerHolder/std::shared_ptr/g {include,libqpdf}/qpdf/*.hh
  402 +patrepl s/PointerHolder/std::shared_ptr/g libqpdf/*.cc
  403 +patrepl s/make_pointer_holder/std::make_shared/g libqpdf/*.cc
  404 +patrepl s/make_array_pointer_holder/QUtil::make_shared_array/g libqpdf/*.cc
  405 +patrepl s,qpdf/std::shared_ptr,qpdf/PointerHolder, **/*.cc **/*.hh
  406 +git restore include/qpdf/PointerHolder.hh
  407 +cleanpatch
405 408  
406   -PointerHolder x(true, new T[5]) -->
407   -auto x = std::shared_ptr(new T[5], std::default_delete<T[]>())
  409 +Increase to POINTERHOLDER_TRANSITION=3
408 410  
409   -X* x = new X(); PointerHolder<X> x_ph(x) -->
410   -auto x_ph = std::make_shared<X>(); X* x = x_ph.get();
  411 +make build_libqpdf -- no errors
411 412  
412   -Derived* x = new Derived(); PointerHolder<Base> x_ph(x) -->
413   -Derived* x = new Derived(); auto x_ph = std::shared_pointer<Base>(x);
  413 +Drop back to POINTERHOLDER_TRANSITION=2
414 414  
415   -Also remember
  415 +make check -- everything passes
416 416  
417   -auto x = std::shared_ptr(new T[5], std::default_delete<T[]>())
418   -vs.
419   -auto x = std::make_unique<T[]>(5)
  417 +Then upgrade everything else. It would work to just start here.
  418 +
  419 +Increase to POINTERHOLDER_TRANSITION=3
  420 +
  421 +patrepl s/PointerHolder/std::shared_ptr/g **/*.cc **/*.hh
  422 +patrepl s/make_pointer_holder/std::make_shared/g **/*.cc
  423 +patrepl s/make_array_pointer_holder/QUtil::make_shared_array/g **/*.cc
  424 +patrepl s,qpdf/std::shared_ptr,qpdf/PointerHolder, **/*.cc **/*.hh
  425 +git restore include/qpdf/PointerHolder.hh
  426 +git restore libtests/pointer_holder.cc
  427 +cleanpatch
  428 +
  429 +Remove all references to PointerHolder.hh from everything except
  430 +public headers and pointer_holder.cc.
  431 +
  432 +make check -- everything passes
  433 +
  434 +Increase to POINTERHOLDER_TRANSITION=4
  435 +
  436 +Do a clean build and make check -- everything passes
  437 +
  438 +Final steps:
  439 +
  440 +* Change to POINTERHOLDER_TRANSITION=4 in autoconf.mk.in.
  441 +* Check code formatting
  442 +* std::shared_ptr<Members> m can be replaced with
  443 + std::shared_ptr<Members> m_ph and Members* m if performance is critical
  444 +* Could try Members indirection with Members* for QPDFObjectHandle
  445 +
  446 +When done:
  447 +
  448 +* Update the smart-pointers section of the manual in design.rst
  449 +* Update comments in PointerHolder.hh
420 450  
421 451 PointerHolder in public API:
422 452  
... ... @@ -453,20 +483,6 @@ PointerHolder in public API:
453 483 QPDFPageObjectHelper::addContentTokenFilter(
454 484 PointerHolder<QPDFObjectHandle::TokenFilter>)
455 485  
456   -Strategy:
457   -* Start with pointerholder branch
458   -* Replace each public API one at a time
459   -* Replace remaining internal uses; sometimes unique_ptr may be good,
460   - particularly for temporary strings that are deleted in the same
461   - scope and Members for non-copyable classes
462   -* std::shared_ptr<Members> m can be replaced with
463   - std::shared_ptr<Members> m_ph and Members* m if performance is critical
464   -* Remove #include <PointerHolder.hh> from all headers
465   -
466   -At that point, we're in a good state to make that compatibility
467   -basically works. Then we can proceed to remove PointerHolder from
468   -everything else.
469   -
470 486 ABI Changes
471 487 ===========
472 488  
... ...
include/qpdf/PointerHolder.hh
... ... @@ -22,13 +22,125 @@
22 22 #ifndef POINTERHOLDER_HH
23 23 #define POINTERHOLDER_HH
24 24  
25   -// In qpdf 11, PointerHolder will be derived from std::shared_ptr and
26   -// will also include a fix to incorrect semantics of const
27   -// PointerHolder objects. PointerHolder only allows a const
28   -// PointerHolder to return a const pointer. This is wrong. Use a
29   -// PointerHolder<const T> for that. A const PointerHolder should just
30   -// not allow you to change what it points to. This is consistent with
31   -// how regular pointers and standard library shared pointers work.
  25 +#ifndef POINTERHOLDER_TRANSITION
  26 +
  27 +// In qpdf 11, #define POINTERHOLDER_IS_SHARED_POINTER
  28 +
  29 +// In qpdf 11, issue a warning:
  30 +// #define POINTERHOLDER_TRANSITION 0 to suppress this warning, and see below.
  31 +// # warn "POINTERHOLDER_TRANSITION is not defined -- see qpdf/PointerHolder.hh"
  32 +
  33 +// undefined = define as 0; will also issue a warning in qpdf 11
  34 +// 0 = no deprecation warnings
  35 +// 1 = make PointerHolder<T>(T*) explicit
  36 +// 2 = warn for use of getPointer() and getRefcount()
  37 +// 3 = warn for all use of PointerHolder
  38 +// 4 = don't define PointerHolder at all
  39 +# define POINTERHOLDER_TRANSITION 0
  40 +#endif // !defined(POINTERHOLDER_TRANSITION)
  41 +
  42 +// *** WHAT IS HAPPENING ***
  43 +
  44 +// In qpdf 11, PointerHolder will be replaced with std::shared_ptr
  45 +// wherever it appears in the qpdf API. The PointerHolder object will
  46 +// be derived from std::shared_ptr to provide a backward-compatible
  47 +// interface and will be mutually assignable with std::shared_ptr.
  48 +// Code that uses containers of PointerHolder will require adjustment.
  49 +
  50 +// *** HOW TO TRANSITION ***
  51 +
  52 +// The symbol POINTERHOLDER_TRANSITION can be defined to help you
  53 +// transition your code away from PointerHolder. You can define it
  54 +// before including any qpdf header files or including its definition
  55 +// in your build configuration. If not defined, it automatically gets
  56 +// defined to 0, which enables full backward compatibility. That way,
  57 +// you don't have to take action for your code to continue to work.
  58 +
  59 +// If you want to work gradually to transition your code away from
  60 +// PointerHolder, you can define POINTERHOLDER_TRANSITION and fix the
  61 +// code so it compiles without warnings and works correctly. If you
  62 +// want to be able to continue to support old qpdf versions at the
  63 +// same time, you can write code like this:
  64 +
  65 +// #ifndef POINTERHOLDER_TRANSITION
  66 +// ... use PointerHolder as before 10.6
  67 +// #else
  68 +// ... use PointerHolder or shared_ptr as needed
  69 +// #endif
  70 +
  71 +// Each level of POINTERHOLDER_TRANSITION exposes differences between
  72 +// PointerHolder and std::shared_ptr. The easiest way to transition is
  73 +// to increase POINTERHOLDER_TRANSITION in steps of 1 so that you can
  74 +// test and handle changes incrementally.
  75 +
  76 +// *** Transitions available starting at qpdf 10.6.0 ***
  77 +
  78 +// POINTERHOLDER_TRANSITION = 1
  79 +//
  80 +// PointerHolder<T> has an implicit constructor that takes a T*, so
  81 +// you can replace a PointerHolder<T>'s pointer by directly assigning
  82 +// a T* to it or pass a T* to a function that expects a
  83 +// PointerHolder<T>. std::shared_ptr does not have this (risky)
  84 +// behavior. When POINTERHOLDER_TRANSITION = 1, PointerHolder<T>'s T*
  85 +// constructor is declared explicit. For compatibility with
  86 +// std::shared_ptr, you can still assign nullptr to a PointerHolder.
  87 +// Constructing all your PointerHolder<T> instances explicitly is
  88 +// backward compatible, so you can make this change without
  89 +// conditional compilation and still use the changes with older qpdf
  90 +// versions.
  91 +//
  92 +// Also defined is a make_pointer_holder method that acts like
  93 +// std::make_shared. You can use this as well, but it is not
  94 +// compatible with qpdf prior to 10.6. Like std::make_shared<T>,
  95 +// make_pointer_holder<T> can only be used when the constructor
  96 +// implied by its arguments is public. If you use this, you should be
  97 +// able to just replace it with std::make_shared when qpdf 11 is out.
  98 +
  99 +// POINTERHOLDER_TRANSITION = 2
  100 +//
  101 +// std::shared_ptr as get() and use_count(). PointerHolder has
  102 +// getPointer() and getRefcount(). In 10.6.0, get() and use_count()
  103 +// were added as well. When POINTERHOLDER_TRANSITION = 2, getPointer()
  104 +// and getRefcount() are deprecated. Fix deprecation warnings by
  105 +// replacing with get() and use_count(). This breaks compatibility
  106 +// with qpdf older than 10.6. Search for CONST BEHAVIOR for an
  107 +// additional note.
  108 +//
  109 +// Once your code is clean at POINTERHOLDER_TRANSITION = 2, the only
  110 +// remaining issues that prevent simple replacement of PointerHolder
  111 +// with std::shared_ptr are shared arrays and containers, and neither
  112 +// of these are used in the qpdf API.
  113 +
  114 +// *** Transitions available starting at qpdf 11.0.0 **
  115 +
  116 +// NOTE: Until qpdf 11 is released, this is a plan and is subject to
  117 +// change. Be sure to check again after qpdf 11 is released.
  118 +
  119 +// POINTERHOLDER_TRANSITION = 3
  120 +//
  121 +// Warn for all use of PointerHolder<T>. This help you remove all use
  122 +// of PointerHolder from your code and use std::shared_ptr instead.
  123 +// You will also have to transition any containers of PointerHolder in
  124 +// your code.
  125 +
  126 +// POINTERHOLDER_TRANSITION = 4
  127 +//
  128 +// Suppress definition of the PointerHolder<T> type entirely.
  129 +
  130 +// CONST BEHAVIOR
  131 +
  132 +// PointerHolder<T> has had a long-standing bug in its const behavior.
  133 +// const PointerHolder<T>'s getPointer() method returns a T const*.
  134 +// This is incorrect and is not how regular pointers or standard
  135 +// library smart pointers behave. Making a PointerHolder<T> const
  136 +// should prevent reassignment of its pointer but not affect the thing
  137 +// it points to. For that, use PointerHolder<T const>. The new get()
  138 +// method behaves correctly in this respect and is therefore slightly
  139 +// different from getPointer(). This shouldn't break any correctly
  140 +// written code. If you are relying on the incorrect behavior, use
  141 +// PointerHolder<T const> instead.
  142 +
  143 +// OLD DOCUMENTATION
32 144  
33 145 // This class is basically std::shared_ptr but predates that by
34 146 // several years.
... ... @@ -58,6 +170,8 @@
58 170 // the underlying pointers provides a well-defined, if not
59 171 // particularly meaningful, ordering.
60 172  
  173 +#include <cstddef>
  174 +
61 175 template <class T>
62 176 class PointerHolder
63 177 {
... ... @@ -65,66 +179,78 @@ class PointerHolder
65 179 class Data
66 180 {
67 181 public:
68   - Data(T* pointer, bool array) :
69   - pointer(pointer),
70   - array(array),
71   - refcount(0)
72   - {
73   - }
74   - ~Data()
75   - {
76   - if (array)
77   - {
78   - delete [] this->pointer;
79   - }
80   - else
81   - {
82   - delete this->pointer;
83   - }
84   - }
85   - T* pointer;
86   - bool array;
87   - int refcount;
  182 + Data(T* pointer, bool array) :
  183 + pointer(pointer),
  184 + array(array),
  185 + refcount(0)
  186 + {
  187 + }
  188 + ~Data()
  189 + {
  190 + if (array)
  191 + {
  192 + delete [] this->pointer;
  193 + }
  194 + else
  195 + {
  196 + delete this->pointer;
  197 + }
  198 + }
  199 + T* pointer;
  200 + bool array;
  201 + int refcount;
88 202 private:
89   - Data(Data const&) = delete;
90   - Data& operator=(Data const&) = delete;
  203 + Data(Data const&) = delete;
  204 + Data& operator=(Data const&) = delete;
91 205 };
92 206  
93 207 public:
  208 +#if POINTERHOLDER_TRANSITION >= 1
  209 + explicit
  210 +#endif // POINTERHOLDER_TRANSITION >= 1
94 211 PointerHolder(T* pointer = 0)
95   - {
96   - this->init(new Data(pointer, false));
97   - }
  212 + {
  213 + this->init(new Data(pointer, false));
  214 + }
98 215 // Special constructor indicating to free memory with delete []
99 216 // instead of delete
100 217 PointerHolder(bool, T* pointer)
101   - {
102   - this->init(new Data(pointer, true));
103   - }
  218 + {
  219 + this->init(new Data(pointer, true));
  220 + }
104 221 PointerHolder(PointerHolder const& rhs)
105   - {
106   - this->copy(rhs);
107   - }
  222 + {
  223 + this->copy(rhs);
  224 + }
108 225 PointerHolder& operator=(PointerHolder const& rhs)
109   - {
110   - if (this != &rhs)
111   - {
112   - this->destroy();
113   - this->copy(rhs);
114   - }
115   - return *this;
116   - }
  226 + {
  227 + if (this != &rhs)
  228 + {
  229 + this->destroy();
  230 + this->copy(rhs);
  231 + }
  232 + return *this;
  233 + }
  234 + PointerHolder& operator=(decltype(nullptr))
  235 + {
  236 + this->operator=(PointerHolder<T>());
  237 + return *this;
  238 + }
117 239 ~PointerHolder()
118   - {
119   - this->destroy();
120   - }
  240 + {
  241 + this->destroy();
  242 + }
121 243 bool operator==(PointerHolder const& rhs) const
122 244 {
123   - return this->data->pointer == rhs.data->pointer;
  245 + return this->data->pointer == rhs.data->pointer;
  246 + }
  247 + bool operator==(decltype(nullptr)) const
  248 + {
  249 + return this->data->pointer == nullptr;
124 250 }
125 251 bool operator<(PointerHolder const& rhs) const
126 252 {
127   - return this->data->pointer < rhs.data->pointer;
  253 + return this->data->pointer < rhs.data->pointer;
128 254 }
129 255  
130 256 // get() is for interface compatibility with std::shared_ptr
... ... @@ -135,33 +261,33 @@ class PointerHolder
135 261  
136 262 // NOTE: The pointer returned by getPointer turns into a pumpkin
137 263 // when the last PointerHolder that contains it disappears.
138   -#ifndef NO_POINTERHOLDER_DEPRECATION
  264 +#if POINTERHOLDER_TRANSITION >= 2
139 265 [[deprecated("use PointerHolder<T>::get() instead of getPointer()")]]
140   -#endif
  266 +#endif // POINTERHOLDER_TRANSITION >= 2
141 267 T* getPointer()
142   - {
143   - return this->data->pointer;
144   - }
145   -#ifndef NO_POINTERHOLDER_DEPRECATION
  268 + {
  269 + return this->data->pointer;
  270 + }
  271 +#if POINTERHOLDER_TRANSITION >= 2
146 272 [[deprecated("use PointerHolder<T>::get() instead of getPointer()")]]
147   -#endif
  273 +#endif // POINTERHOLDER_TRANSITION >= 2
148 274 T const* getPointer() const
149   - {
150   - return this->data->pointer;
151   - }
152   -#ifndef NO_POINTERHOLDER_DEPRECATION
  275 + {
  276 + return this->data->pointer;
  277 + }
  278 +#if POINTERHOLDER_TRANSITION >= 2
153 279 [[deprecated("use use_count() instead of getRefcount()")]]
154   -#endif
  280 +#endif // POINTERHOLDER_TRANSITION >= 2
155 281 int getRefcount() const
156   - {
157   - return this->data->refcount;
158   - }
  282 + {
  283 + return this->data->refcount;
  284 + }
159 285  
160 286 // use_count() is for compatibility with std::shared_ptr
161 287 long use_count()
162   - {
163   - return static_cast<long>(this->data->refcount);
164   - }
  288 + {
  289 + return static_cast<long>(this->data->refcount);
  290 + }
165 291  
166 292 T const& operator*() const
167 293 {
... ... @@ -183,30 +309,44 @@ class PointerHolder
183 309  
184 310 private:
185 311 void init(Data* data)
186   - {
187   - this->data = data;
188   - ++this->data->refcount;
189   - }
  312 + {
  313 + this->data = data;
  314 + ++this->data->refcount;
  315 + }
190 316 void copy(PointerHolder const& rhs)
191   - {
192   - this->init(rhs.data);
193   - }
  317 + {
  318 + this->init(rhs.data);
  319 + }
194 320 void destroy()
195   - {
196   - bool gone = false;
197   - {
198   - if (--this->data->refcount == 0)
199   - {
200   - gone = true;
201   - }
202   - }
203   - if (gone)
204   - {
205   - delete this->data;
206   - }
207   - }
  321 + {
  322 + bool gone = false;
  323 + {
  324 + if (--this->data->refcount == 0)
  325 + {
  326 + gone = true;
  327 + }
  328 + }
  329 + if (gone)
  330 + {
  331 + delete this->data;
  332 + }
  333 + }
208 334  
209 335 Data* data;
210 336 };
211 337  
  338 +template<typename T, typename... _Args>
  339 +inline PointerHolder<T>
  340 +make_pointer_holder(_Args&&... __args)
  341 +{
  342 + return PointerHolder<T>(new T(__args...));
  343 +}
  344 +
  345 +template <typename T>
  346 +PointerHolder<T>
  347 +make_array_pointer_holder(size_t n)
  348 +{
  349 + return PointerHolder<T>(true, new T[n]);
  350 +}
  351 +
212 352 #endif // POINTERHOLDER_HH
... ...
libtests/pointer_holder.cc
1   -#define NO_POINTERHOLDER_DEPRECATION // we need to test the deprecated API
  1 +// We need to test the deprecated API
  2 +#ifdef POINTERHOLDER_TRANSITION
  3 +# undef POINTERHOLDER_TRANSITION
  4 +#endif
  5 +#define POINTERHOLDER_TRANSITION 0
2 6 #include <qpdf/PointerHolder.hh>
3 7  
4 8 #include <iostream>
... ...
manual/design.rst
... ... @@ -751,16 +751,26 @@ actually quite rare and largely avoidable.
751 751 Smart Pointers
752 752 --------------
753 753  
754   -This section describes changes to the use of smart pointers in qpdf in
755   -versions 10.6.0 and 11.0.0.
  754 +This section describes changes to the use of smart pointers there were
  755 +made in qpdf 10.6.0 as well as some planned for 11.0.0.
756 756  
757 757 Starting in qpdf 11, ``PointerHolder`` will be replaced with
758 758 ``std::shared_ptr`` in qpdf's public API. A backward-compatible
759   -``PointerHolder`` will be provided that should make it possible for
760   -most code to remain unchanged. This new ``PointerHolder`` will be
761   -marked deprecated but will provide a way to suppress the deprecation
762   -warnings. Code that works with containers of ``PointerHolder`` may
763   -have to be modified, though no qpdf interfaces do this.
  759 +``PointerHolder`` class will be provided that should make it possible
  760 +for most code to remain unchanged. ``PointerHolder`` may eventually be
  761 +removed from qpdf entirely, but this will not happen for a while to
  762 +make it easier for people who need to support multiple versions of
  763 +qpdf.
  764 +
  765 +The ``POINTERHOLDER_TRANSITION`` preprocessor symbol has been
  766 +introduced to help people transition from ``PointerHolder`` to
  767 +``std::shared_ptr``. After qpdf 11 is released, to prepare for a
  768 +future qpdf without ``PointerHolder`` and to let them know that it is
  769 +no longer needed, a warning will be issued if
  770 +``<qpdf/PointerHolder.hh>`` is included, though it will be possible to
  771 +suppress the warning by defining ``POINTERHOLDER_TRANSITION``. In
  772 +10.6.0, there are some steps you can perform to prepare, but no action
  773 +is required.
764 774  
765 775 The remainder of this section describes how to prepare if you want to
766 776 eliminate ``PointerHolder`` from your code or what to do if you want
... ... @@ -769,25 +779,31 @@ to stick with the old interfaces.
769 779 Changes in 10.6.0
770 780 ~~~~~~~~~~~~~~~~~
771 781  
772   -In qpdf 10.6.0, two ``PointerHolder`` methods have been deprecated and
773   -replaced with methods that are compatible with ``std::shared_ptr``:
  782 +In qpdf 10.6.0, the following changes have been made to
  783 +``PointerHolder`` to make its behavior closer to that of
  784 +``std::shared_ptr``:
774 785  
775   -- ``getPointer()`` -- use ``get()`` instead
  786 +- ``get()`` has been added as an alternative to ``getPointer()``
776 787  
777   -- ``getRefcount()`` -- use ``use_count()`` instead
  788 +- ``use_count()`` has been added as an alternative to ``getRefcount()``
778 789  
779   -If you build your code with deprecation warnings enabled and you want
780   -to suppress these deprecation warnings for now, you can ``#define
781   -NO_POINTERHOLDER_DEPRECATION`` before including any qpdf header files.
782   -It may be possible to leave it this way long-term to facilitate
783   -supporting older versions of qpdf without conditional compilation.
  790 +- A new global helper function ``make_pointer_holder`` behaves
  791 + similarly to ``std::make_shared``, so you can use
  792 + ``make_pointer_holder<T>(args...)`` to create a ``PointerHolder<T>``
  793 + with ``new T(args...)`` as the pointer.
  794 +
  795 +- A new global helper function ``make_array_pointer_holder`` takes a
  796 + size and creates a ``PointerHolder`` to an array. It is a
  797 + counterpart to the newly added ``QUtil::make_shared_array`` method,
  798 + which does the same thing with a ``std::shared_ptr``.
784 799  
785 800 ``PointerHolder`` has had a long-standing bug: a ``const
786 801 PointerHolder<T>`` would only provide a ``T const*`` with its
787   -``getPointer`` method. This is incorrect and is not how standard C++
788   -smart pointers or regular pointers behave. The correct semantics
789   -would be that a ``const PointerHolder<T>`` would not accept a new
790   -pointer after being created but would still allow you to modify the
  802 +``getPointer`` method. This is incorrect and is not how standard
  803 +library C++ smart pointers or regular pointers behave. The correct
  804 +semantics would be that a ``const PointerHolder<T>`` would not accept
  805 +a new pointer after being created (``PointerHolder`` has always
  806 +behaved correctly in this way) but would still allow you to modify the
791 807 item being pointed to. If you don't want to mutate the thing it points
792 808 to, use ``PointerHolder<T const>`` instead. The new ``get()`` method
793 809 behaves correctly. It is therefore not exactly the same as
... ... @@ -795,157 +811,290 @@ behaves correctly. It is therefore not exactly the same as
795 811 ``std::shared_ptr``. This shouldn't make any difference to any
796 812 correctly written code.
797 813  
  814 +Differences between ``PointerHolder`` and ``std::shared_ptr``
  815 +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  816 +
  817 +Here is a list of things you need to think about when migrating from
  818 +``PointerHolder`` to ``std::shared_ptr``. After the list, we will
  819 +discuss how to address each one using the ``POINTERHOLDER_TRANSITION``
  820 +preprocessor symbol or other C++ coding techniques.
  821 +
  822 +- ``PointerHolder<T>`` has an *implicit* constructor that takes a
  823 + ``T*``, which means you can assign a ``T*`` directly to a
  824 + ``PointerHolder<T>`` or pass a ``T*`` to a function that expects a
  825 + ``PointerHolder<T>`` as a parameter. ``std::shared_ptr<T>`` does not
  826 + have this behavior, though you can still assign ``nullptr`` to a
  827 + ``std::shared_ptr<T>`` and compare ``nullptr`` with a
  828 + ``std::shared_ptr<T>``. Here are some examples of how you might need
  829 + to change your code:
  830 +
  831 + Old code:
  832 + .. code-block:: c++
  833 +
  834 + PointerHolder<X> x_p;
  835 + X* x = new X();
  836 + x_p = x;
  837 +
  838 + New code:
  839 + .. code-block:: c++
  840 +
  841 + auto x_p = std::make_shared<X>();
  842 + X* x = x_p.get();
  843 + // or, less safe, but closer:
  844 + std::shared_ptr<X> x_p;
  845 + X* x = new X();
  846 + x_p = std::shared_ptr<X>(x);
  847 +
  848 + Old code:
  849 + .. code-block:: c++
  850 +
  851 + PointerHolder<Base> base_p;
  852 + Derived* derived = new Derived();
  853 + base_p = derived;
  854 +
  855 + New code:
  856 + .. code-block:: c++
  857 +
  858 + std::shared_ptr<Base> base_p;
  859 + Derived* derived = new Derived();
  860 + base_p = std::shared_ptr<Base>(derived);
  861 +
  862 +- ``PointerHolder<T>`` has ``getPointer()`` to get the underlying
  863 + pointer. It also has the seldom-used ``getRefcount()`` method to get
  864 + the reference count. ``std::shared_ptr<T>`` has ``get()`` and
  865 + ``use_count()``. In qpdf 10.6, ``PointerHolder<T>`` also has
  866 + would not be an issue unless you did this in your own code.
  867 +
  868 +Addressing the Differences
  869 +~~~~~~~~~~~~~~~~~~~~~~~~~~
  870 +
  871 +If you need to support versions of qpdf prior to qpdf 10.6, you don't
  872 +*need* to take any action at this time, but it is recommended that you
  873 +at least address the implicit constructor issue since this can be done
  874 +without breaking backward compatibility. (Explicit construction of
  875 +``PointerHolder<T>`` is and always has been allowed.)
  876 +
  877 +There are two significant things you can do to minimize the impact of
  878 +switching from ``PointerHolder`` to ``std::shared_ptr``:
  879 +
  880 +- Use ``auto`` and ``decltype`` whenever possible when working with
  881 + ``PointerHolder`` variables that are exchanged with the qpdf API.
  882 +
  883 +- Use the ``POINTERHOLDER_TRANSITION`` preprocessor symbol to identify
  884 + and resolve the differences described above.
  885 +
  886 +To use ``POINTERHOLDER_TRANSITION``, you will need to ``#define`` it
  887 +before including any qpdf header files or specify its value as part of
  888 +your build. The table below describes the values of
  889 +``POINTERHOLDER_TRANSITION``. This informatoin is also summarized in
  890 +:file:`include/qpdf/PointerHolder.hh`, so you will have it handy
  891 +without consulting this manual.
  892 +
  893 +.. list-table:: POINTERHOLDER_TRANSITION values
  894 + :widths: 5 80
  895 + :header-rows: 1
  896 +
  897 + - - value
  898 + - meaning
  899 +
  900 + - - undefined
  901 + - same as ``0``, but start with qpdf 11.0, issues a warning
  902 +
  903 + - - ``0``
  904 + - provide a backward compatible ``PointerHolder`` and suppress
  905 + all deprecation warnings
  906 +
  907 + - - ``1``
  908 + - Make the ``PointerHolder<T>(T*)`` constructor explicit
  909 +
  910 + - - ``2``
  911 + - Deprecate ``getPointer()`` and ``getRefcount()``
  912 +
  913 + - - ``3``
  914 + - Starting in qpdf 11, deprecate all uses of ``PointerHolder``
  915 +
  916 + - - ``4``
  917 + - Starting in qpdf 11, disable all functionality from
  918 + ``qpdf/PointerHolder.hh`` so that ``#include``-ing it has no
  919 + effect.
  920 +
  921 +Based on the above, here is a procedure for preparing your code. This
  922 +is the procedure that was used for the qpdf code itself.
  923 +
  924 +If you need to support versions of qpdf prior to 10.6, you can still
  925 +do these steps:
  926 +
  927 +- Find all occurrences of ``PointerHolder`` in the code. See whether
  928 + any of them can just be outright replaced with ``std::shared_ptr``
  929 + or ``std::unique_ptr``. If you have been using qpdf prior to
  930 + adopting C++11 and were using ``PointerHolder`` as a general-purpose
  931 + smart pointer, you may have cases that can be replaced in this way.
  932 +
  933 + For example:
  934 +
  935 + - Simple ``PointerHolder<T>`` construction can be replaced with
  936 + either the equivalent ``std::shared_ptr<T>`` construction or, if
  937 + the constructor is public, with ``std::make_shared<T>(args...)``.
  938 + If you are creating a smart pointer that is never copied, you may
  939 + be able to use ``std::unique_ptr<T>`` instead.
  940 +
  941 + - Array allocations will have to be rewritten.
798 942  
799   -How to Prepare
800   -~~~~~~~~~~~~~~
  943 + Allocating a ``PointerHolder`` to an array looked like this:
801 944  
802   -If you don't need to support versions of qpdf prior to 10.6, you can
803   -just replace all occurrences of ``getPointer()`` with ``get()`` and
804   -all occurrences of ``getRefcount()`` with ``use_count()``. That's
805   -about all you will be able to do prior to qpdf 11.
  945 + .. code-block:: c++
806 946  
807   -If you need to support older versions, you have two choices:
  947 + PointerHolder<X> p(true, new X[n]);
808 948  
809   -- ``#define NO_POINTERHOLDER_DEPRECATION`` and leave everything the
810   - way it was. You can just wait until qpdf 11.
  949 + To allocate a ``std::shared_ptr`` to an array:
811 950  
812   -- Write code that uses ``get()`` but falls back to ``getPointer()`` if
813   - ``QPDF_MAJOR_VERSION`` is not defined. The symbols
814   - ``QPDF_MAJOR_VERSION``, ``QPDF_MINOR_VERSION``, and
815   - ``QPDF_PATCH_VERSION`` were introduced with 10.6.0, so just checking
816   - for whether ``QPDF_MAJOR_VERSION`` is defined is sufficient for
817   - telling if you're running a version before 10.6.0. If you do this,
818   - once qpdf 11 comes out, you will already know all the places that
819   - have to be handled specially.
  951 + .. code-block:: c++
820 952  
821   -If you are somehow relying on the fact that a ``const
822   -PointerHolder<T>`` always gave back a ``T const*`` and are
823   -dereferencing a ``const PointerHolder<T>`` to call methods that only
824   -have ``const`` versions in ``T``, you may have to change from
825   -``const PointerHolder<T>`` to ``PointerHolder<T const>``. This won't
826   -be an issue for anything in the qpdf API, and if you are using qpdf
827   -``PointerHolder`` objects for any other reason, you should just
828   -replace them with ``std::shared_ptr``.
  953 + auto p = std::shared_ptr<X>(new X[n], std::default_delete<X[]>());
  954 + // If you don't mind using QUtil, there's QUtil::make_shared_array<X>(n).
  955 + // If you are using c++20, you can use std::make_shared<X[]>(n)
  956 + // to get a std::shared_ptr<X[]> instead of a std::shared_ptr<X>.
829 957  
830   -What to Expect
831   -~~~~~~~~~~~~~~
  958 + To allocate a ``std::unique_ptr`` to an array:
832 959  
833   -Note: if you are reading this in the 10.6 manual and 11 is out, you
834   -should read it in the manual for qpdf 11 instead. Some early tests
835   -have been done to try to ensure the accuracy of this information, but
836   -it may change once the work is actually completed.
  960 + .. code-block:: c++
837 961  
838   -When ``PointerHolder`` disappears from qpdf's API in qpdf 11, you will
839   -have a few options:
  962 + auto p = std::make_unique<X[]>(n);
  963 + // or, if X has a private constructor:
  964 + auto p = std::unique_ptr<X[]>(new X[n]);
840 965  
841   -- Use the new ``PointerHolder``, which is derived from
842   - ``std::shared_ptr`` and which has methods to make it
843   - interchangeable. For things that use ``PointerHolder<T>`` directly,
844   - this should "just work," though you will have to ``#define
845   - NO_POINTERHOLDER_DEPRECATION`` if you don't want deprecation
846   - warnings.
  966 +- If a ``PointerHolder<T>`` can't be replaced with a standard library
  967 + smart pointer, perhaps it can be declared using ``auto`` or
  968 + ``decltype`` so that, when the qpdf API changes, your code will just
  969 + need to be recompiled.
847 970  
848   -- Replace all uses of ``PointerHolder<T>`` with ``std::shared_ptr<T>``
849   - and deal with the required changes, outlined below. This is the
850   - recommended course of action. You will need conditional compilation
851   - if you want to simultaneously support order code. Stay tuned for the
852   - qpdf 11 documentation for specifics.
  971 +- ``#define POINTERHOLDER_TRANSITION 1`` to enable deprecation
  972 + warnings for all implicit constructions of ``PointerHolder<T>`` from
  973 + a plain ``T*``. When you find one, explicitly construct the
  974 + ``PointerHolder<T>``.
853 975  
854   -While ``PointerHolder<T>`` and ``std::shared_ptr<T>`` will be mutually
855   -assignable and convertible, this does not apply to containers of those
856   -objects. The qpdf API doesn't have any containers of
857   -``PointerHolder``, so this would have to be in your own code. You can
858   -prepare yourself for the change by using ``auto`` and ``decltype``
859   -whenever possible so that a change to the underlying type of something
860   -won't require source changes.
  976 + - Old code:
861 977  
862   -Required Changes in qpdf 11
863   -~~~~~~~~~~~~~~~~~~~~~~~~~~~
  978 + .. code-block:: c++
864 979  
865   -This section describes unavoidable changes when replacing
866   -``PointerHolder`` with ``std::shared_ptr`` rather than continuing to
867   -use the backward compatible API. Nothing here is needed (or can be
868   -done) prior to qpdf 11, so consider this to be a preview.
  980 + PointerHolder<X> x = new X();
869 981  
870   -- Change ``getPointer`` to ``get`` and ``getRefcount`` to
871   - ``use_count`` as above. If your starting point is no deprecation
872   - warnings with qpdf 10.6, this will already be true.
  982 + - New code:
873 983  
874   -- Array allocations will have to be rewritten.
  984 + .. code-block:: c++
875 985  
876   - To allocate a ``PointerHolder`` to an array:
  986 + auto x = PointerHolder<X>(new X(...)); // all versions of qpdf
  987 + // or, if X(...) is public:
  988 + auto x = make_pointer_holder<X>(...); // only 10.6 and above
877 989  
878   - .. code-block:: c++
  990 + Other examples appear above.
879 991  
880   - PointerHolder<X> p(true, new X[n]);
  992 +If you need to support older versions of qpdf than 10.6, this is as
  993 +far as you can go until qpdf 11 comes out.
881 994  
882   - To allocate a ``std::shared_ptr`` to an array:
  995 +If you only need to support the latest version of qpdf, proceed as
  996 +follows:
883 997  
884   - .. code-block:: c++
  998 +- ``#define POINTERHOLDER_TRANSITION 2`` to enable deprecation of
  999 + ``getPointer()`` and ``getRefcount()``
885 1000  
886   - auto p = std::shared_ptr<X>(new X[n], std::default_delete<X[]>());
  1001 +- Replace ``getPointer()`` with ``get()`` and ``getRefcount()`` with
  1002 + ``use_count()``. These methods were not present prior to 10.6.0.
887 1003  
888   - To allocate a ``std::unique_ptr`` to an array:
  1004 +When you have gotten your code to compile cleanly with
  1005 +``POINTERHOLDER_TRANSITION=2``, you are well on your way to being
  1006 +ready for eliminating ``PointerHolder`` entirely after qpdf 11 is
  1007 +released.
889 1008  
890   - .. code-block:: c++
  1009 +After qpdf 11 is out
  1010 +~~~~~~~~~~~~~~~~~~~~
891 1011  
892   - auto p = std::make_unique<X[]>(n);
893   - // or
894   - auto p = std::unique_ptr<X[]>(new X[n]);
  1012 +In the 10.6 manual, this section represents a plan and is subject to
  1013 +change. However, it has been tested in practice using a version of the
  1014 +qpdf 11 ``PointerHolder`` on a branch, so it is likely to be accurate.
  1015 +In the meantime, think of this is a preview.
895 1016  
896   - The second form may be needed if ``X`` has a private constructor
897   - from this context.
  1017 +First, make sure you have done the steps in the 10.6 section. (Note:
  1018 +once qpdf 11 comes out, the goal is to not have to migrate to 10.6
  1019 +first, so it is likely that these sections will be combined.)
898 1020  
899   - C++-17 has a better way to allocate ``std::shared_ptr`` to an array,
900   - but qpdf is still allowing C++-14 to be used. You can use whatever
901   - method to handle shared arrays that is supported in your
902   - environment. There are no shared arrays in qpdf's public API except
903   - for some ``QUtil`` helper methods that are not essential for use of
904   - qpdf features.
  1021 +If you are explicitly choosing to stick with the backward compatible
  1022 +``PointerHolder`` for now, you should define
  1023 +``POINTERHOLDER_TRANSITION`` to ``0`` to suppress the warning from
  1024 +including ``qpdf/PointerHolder.hh``. Be aware that you may eventually
  1025 +have to deal with the transition, though the intention is to leave the
  1026 +compatibility layer in place for a while. You should rebuild and test
  1027 +your code. There may be compiler errors if you have containers of
  1028 +``PointerHolder``, but most code should compile without any changes.
  1029 +Even if you have errors, use of ``auto`` or ``decltype`` may enable
  1030 +you to write code that works with the old and new API without having
  1031 +to use conditional compilation. The
  1032 +``POINTERHOLDER_IS_SHARED_POINTER`` is defined in qpdf 11 if you
  1033 +``#include <qpdf/PointerHolder.hh>``.
905 1034  
906   -- ``PointerHolder<T>`` can have plain pointers directly assigned to
907   - it, while ``std::shared_ptr<T>`` cannot. This makes code like this
908   - possible:
  1035 +If you want to support older versions of qpdf and still transition so
  1036 +that the backward-compatible ``PointerHolder`` is not in use, you can
  1037 +separate old code and new code by testing with the
  1038 +``POINTERHOLDER_IS_SHARED_POINTER`` preprocessor symbol, as in
909 1039  
910   - .. code-block:: c++
  1040 +.. code-block:: c++
911 1041  
912   - PointerHolder<X> x_p;
913   - X* x = new X();
914   - x_p = x;
  1042 + #ifdef POINTERHOLDER_IS_SHARED_POINTER
  1043 + std::shared_ptr<X> x;
  1044 + #else
  1045 + PointerHolder<X> x;
  1046 + #endif // POINTERHOLDER_IS_SHARED_POINTER
  1047 + x = decltype(x)(new X())
915 1048  
916   - It also makes it possible to pass a plain pointer to a function
917   - expecting a ``PointerHolder``, thereby transferring "ownership" of
918   - the pointer into the function.
  1049 +or
919 1050  
920   - Code like that is a risky because you can leak memory if an
921   - exception is thrown between creation of the X and assignment of it
922   - into the ``PointerHolder``. In any case, ``std::shared_ptr`` does
923   - not allow that, so you need one of these instead:
  1051 +.. code-block:: c++
924 1052  
925   - .. code-block:: c++
  1053 + #ifdef POINTERHOLDER_IS_SHARED_POINTER
  1054 + auto x_p = std::make_shared<X>();
  1055 + X* x = x_p.get();
  1056 + #else
  1057 + auto x_p = PointerHolder<X>(new X());
  1058 + X* x = x_p.getPointer();
  1059 + #endif // POINTERHOLDER_IS_SHARED_POINTER
  1060 + x_p->doSomething();
  1061 + x->doSomethingElse();
926 1062  
927   - auto x_p = std::make_shared<X>();
928   - X* x = x_p.get();
929   - // or, less safe, but closer:
930   - std::shared_ptr<X> x_p;
931   - X* x = new X();
932   - x_p = std::shared_ptr<X>(x);
  1063 +If you don't need to support older versions of qpdf, you can proceed
  1064 +with these steps without protecting changes with the preprocessor
  1065 +symbol. Here are the remaining changes.
933 1066  
934   - Also, code like this:
  1067 +- Make sure you have a clean build with ``POINTERHOLDER_TRANSITION``
  1068 + set to ``2``. This means that you are using ``PointerHolder`` in a
  1069 + manner that is API-compatible with ``std::shared_ptr`` in all cases
  1070 + except for array pointers.
935 1071  
936   - .. code-block:: c++
  1072 +- Replace all occurrences of ``PointerHolder`` with
  1073 + ``std::shared_ptr`` except in ``#include <qpdf/PointerHolder.hh>``
937 1074  
938   - PointerHolder<Base> base_p;
939   - Derived* derived = new Derived();
940   - base_p = derived;
  1075 +- Replace all occurrences of ``make_pointer_holder`` with
  1076 + ``std::make_shared``
941 1077  
942   - needs to be replaced with something like this instead:
  1078 +- Replace all occurrences of ``make_array_pointer_holder`` with
  1079 + ``QUtil::make_shared_array``. You will need to include
  1080 + ``<qpdf/QUtil.hh>`` if you haven't already done so.
943 1081  
944   - .. code-block:: c++
  1082 +- Make sure ``<memory>`` is included wherever you were including
  1083 + ``<qpdf/PointerHolder.hh>``.
945 1084  
946   - std::shared_ptr<Base> base_p;
947   - Derived* derived = new Derived();
948   - base_p = std::shared_ptr<Base>(derived);
  1085 +- If you were using any array ``PointerHolder<T>`` objects, replace
  1086 + them as above. You can let the compiler find these for you.
  1087 +
  1088 +- ``#define POINTERHOLDER_TRANSITION 3`` to enable deprecation of
  1089 + all ``PointerHolder<T>`` construction.
  1090 +
  1091 +- Build and test. Fix any remaining issues.
  1092 +
  1093 +- If not supporting older qpdf, remove all references to
  1094 + ``<qpdf/PointerHolder.hh>``. Otherwise, you wil still need to
  1095 + include it but can ``#define POINTERHOLDER_TRANSITION 4`` to prevent
  1096 + ``PointerHolder`` from being defined. The
  1097 + ``POINTERHOLDER_IS_SHARED_POINTER`` symbol will still be defined.
949 1098  
950 1099 Historical Background
951 1100 ~~~~~~~~~~~~~~~~~~~~~
... ... @@ -953,13 +1102,6 @@ Historical Background
953 1102 Since its inception, the qpdf library used its own smart pointer
954 1103 class, ``PointerHolder``. The ``PointerHolder`` class was originally
955 1104 created long before ``std::shared_ptr`` existed, and qpdf itself
956   -didn't start requiring a C++-11 compiler version 9.1.0 released in
957   -late 2019.
958   -
959   -``PointerHolder`` is a reference-counted smart pointer with semantics
960   -almost identical to ``std::shared_ptr`` except that it is not
961   -thread-safe. It has a few interface differences that prevent
962   -``std::shared_ptr`` from being a drop-in replacement. However, given
963   -the value of using standard library smart pointers, qpdf is taking the
964   -plunge for version 11 and switching over to standard library smart
965   -pointers.
  1105 +didn't start requiring a C++11 compiler version 9.1.0 released in
  1106 +late 2019. With current C++ versions, is no longer desirable for qpdf
  1107 +to have its own smart pointer class.
... ...
manual/release-notes.rst
... ... @@ -7,28 +7,15 @@ For a detailed list of changes, please see the file
7 7 :file:`ChangeLog` in the source distribution.
8 8  
9 9 10.6.0: XXX
10   - - Deprecations/future replacement of ``PointerHolder``
  10 + - Preparation for replacement of ``PointerHolder``
11 11  
12 12 The next major release of qpdf will replace ``PointerHolder`` with
13   - ``std::shared_ptr`` across all of qpdf's public API. In
14   - preparation for this change, the following ``PointerHolder``
15   - methods have been deprecated in favor of interfaces that more
16   - closely match ``std::shared_ptr``:
17   -
18   - - ``getPointer()`` -- use ``get()`` instead; this also fixes
19   - ``const`` semantics as discussed in
20   - :file:`include/qpdf/PointerHolder.hh`.
21   -
22   - - ``getRefcount()`` -- use ``use_count()`` instead
23   -
24   - If you build your code with deprecation warnings enabled and you
25   - want to suppress these deprecation warnings for now, you can
26   - ``#define NO_POINTERHOLDER_DEPRECATION`` before including any qpdf
27   - header files. Code that does this will *require no changes* prior
28   - to qpdf 11 and may or may not require changes after qpdf 11.
29   -
30   - For a detailed discussion of this change and how to prepare for
31   - it, see :ref:`smart-pointers`.
  13 + ``std::shared_ptr`` across all of qpdf's public API. No action is
  14 + required at this time, but if you'd like to prepare, read the
  15 + comments :file:`include/qpdf/PointerHolder.hh` and see
  16 + :ref:`smart-pointers` for details on what you can do now to create
  17 + code that will continue to work with older versions of qpdf and be
  18 + easier to switch over to qpdf 11 when it comes out.
32 19  
33 20 - Preparation for a new JSON output version
34 21  
... ...