Commit e782d5e951e33b31068e0e71e7ce99a88b882544
1 parent
e58b1174
TODO: update notes about PointerHolder
Showing
1 changed file
with
77 additions
and
17 deletions
TODO
| @@ -20,20 +20,14 @@ | @@ -20,20 +20,14 @@ | ||
| 20 | or crypt provider as long as this can be done in a thread-safe | 20 | or crypt provider as long as this can be done in a thread-safe |
| 21 | fashion. | 21 | fashion. |
| 22 | 22 | ||
| 23 | -* Completion: would be nice if --job-json-file=<TAB> would complete | ||
| 24 | - files | ||
| 25 | - | ||
| 26 | * Remember for release notes: starting in qpdf 11, the default value | 23 | * Remember for release notes: starting in qpdf 11, the default value |
| 27 | for the --json keyword will be "latest". If you are depending on | 24 | for the --json keyword will be "latest". If you are depending on |
| 28 | version 1, change your code to specify --json=1, which works | 25 | version 1, change your code to specify --json=1, which works |
| 29 | starting with 10.6.0. | 26 | starting with 10.6.0. |
| 30 | 27 | ||
| 31 | -* Try to put something in to ease future PointerHolder migration, such | ||
| 32 | - as typedefs for containers of PointerHolders. Test to see whether | ||
| 33 | - using auto or decltype in certain places may make containers of | ||
| 34 | - pointerholders switch over cleanly. Clearly document the deprecation | ||
| 35 | - stuff. | ||
| 36 | - | 28 | +* Write up something about preparing for the PointerHolder to |
| 29 | + shared_ptr migration. Clearly document the deprecations and how to | ||
| 30 | + deal with them. | ||
| 37 | 31 | ||
| 38 | Output JSON v2 | 32 | Output JSON v2 |
| 39 | ============== | 33 | ============== |
| @@ -345,6 +339,79 @@ Other notes: | @@ -345,6 +339,79 @@ Other notes: | ||
| 345 | way that works for the qpdf/qpdf repository as well since they are | 339 | way that works for the qpdf/qpdf repository as well since they are |
| 346 | very similar. | 340 | very similar. |
| 347 | 341 | ||
| 342 | + | ||
| 343 | +PointerHolder to std::shared_ptr | ||
| 344 | +================================ | ||
| 345 | + | ||
| 346 | +Once all deprecation warnings are cleared up (changing getPointer() to | ||
| 347 | +get() and getRefcout() to use_count()), the only real issues are that | ||
| 348 | +implicit assignment of a pointer to a shared_ptr doesn't work while it | ||
| 349 | +does for PointerHolder and containers are different. Using auto takes | ||
| 350 | +care of containers. | ||
| 351 | + | ||
| 352 | +PointerHolder<X> x = new X() --> | ||
| 353 | +auto x = std::make_shared<X>() | ||
| 354 | + | ||
| 355 | +PointerHolder<Base> x = new Derived() --> | ||
| 356 | +auto x = std::shared_ptr<Base>(new Derived()) | ||
| 357 | + | ||
| 358 | +PointerHolder x(true, new T[5]) --> | ||
| 359 | +auto x = std::shared_ptr(new T[5], std::default_delete<T[]>()) | ||
| 360 | + | ||
| 361 | +X* x = new X(); PointerHolder<X> x_ph(x) --> | ||
| 362 | +auto x_ph = std::make_shared<X>(); X* x = x_ph.get(); | ||
| 363 | + | ||
| 364 | +Derived* x = new Derived(); PointerHolder<Base> x_ph(x) --> | ||
| 365 | +Derived* x = new Derived(); auto x_ph = std::shared_pointer<Base>(x); | ||
| 366 | + | ||
| 367 | +PointerHolder in public API: | ||
| 368 | + | ||
| 369 | + QUtil::read_file_into_memory( | ||
| 370 | + char const*, PointerHolder<char>&, unsigned long&) | ||
| 371 | + QPDFObjectHandle::addContentTokenFilter( | ||
| 372 | + PointerHolder<QPDFObjectHandle::TokenFilter>) | ||
| 373 | + QPDFObjectHandle::addTokenFilter( | ||
| 374 | + PointerHolder<QPDFObjectHandle::TokenFilter>) | ||
| 375 | + QPDFObjectHandle::newStream( | ||
| 376 | + QPDF*, PointerHolder<Buffer>) | ||
| 377 | + QPDFObjectHandle::parse( | ||
| 378 | + PointerHolder<InputSource>, std::string const&, | ||
| 379 | + QPDFTokenizer&, bool&, QPDFObjectHandle::StringDecrypter*, QPDF*) | ||
| 380 | + QPDFObjectHandle::replaceStreamData( | ||
| 381 | + PointerHolder<Buffer>, QPDFObjectHandle const&, | ||
| 382 | + QPDFObjectHandle const&) | ||
| 383 | + QPDFObjectHandle::replaceStreamData( | ||
| 384 | + PointerHolder<QPDFObjectHandle::StreamDataProvider>, | ||
| 385 | + QPDFObjectHandle const&, QPDFObjectHandle const&) | ||
| 386 | + QPDFTokenizer::expectInlineImage( | ||
| 387 | + PointerHolder<InputSource>) | ||
| 388 | + QPDFTokenizer::readToken( | ||
| 389 | + PointerHolder<InputSource>, std::string const&, | ||
| 390 | + bool, unsigned long) | ||
| 391 | + QPDF::processInputSource( | ||
| 392 | + PointerHolder<InputSource>, char const*) | ||
| 393 | + QPDFWriter::registerProgressReporter( | ||
| 394 | + PointerHolder<QPDFWriter::ProgressReporter>) | ||
| 395 | + QPDFEFStreamObjectHelper::createEFStream( | ||
| 396 | + QPDF&, PointerHolder<Buffer>) | ||
| 397 | + QPDFPageObjectHelper::addContentTokenFilter( | ||
| 398 | + PointerHolder<QPDFObjectHandle::TokenFilter>) | ||
| 399 | + | ||
| 400 | +Strategy: | ||
| 401 | +* Start with pointerholder branch | ||
| 402 | +* Replace each public API one at a time | ||
| 403 | +* Replace remaining internal uses; sometimes unique_ptr may be good, | ||
| 404 | + particularly for temporary strings that are deleted in the same | ||
| 405 | + scope and Members for non-copiable classes | ||
| 406 | +* std::shared_ptr<Members> m can be replaced with | ||
| 407 | + std::shared_ptr<Members> m_ph and Members* m if performance is critical | ||
| 408 | +* Remove #include <PointerHolder.hh> from all headers | ||
| 409 | + | ||
| 410 | +At that point, we're in a good state to make that compatibility | ||
| 411 | +basically works. Then we can proceed to remove PointerHolder from | ||
| 412 | +everything else. | ||
| 413 | + | ||
| 414 | + | ||
| 348 | ABI Changes | 415 | ABI Changes |
| 349 | =========== | 416 | =========== |
| 350 | 417 | ||
| @@ -353,14 +420,7 @@ Comments appear in the code prefixed by "ABI" | @@ -353,14 +420,7 @@ Comments appear in the code prefixed by "ABI" | ||
| 353 | 420 | ||
| 354 | * Search for ABI to find items not listed here. | 421 | * Search for ABI to find items not listed here. |
| 355 | * Switch default --json to latest | 422 | * Switch default --json to latest |
| 356 | -* Take a fresh look at PointerHolder with a good plan for being able | ||
| 357 | - to have developers phase it in using macros or something. Decide | ||
| 358 | - about shared_ptr vs unique_ptr for each time make_shared_cstr is | ||
| 359 | - called. For non-copiable classes, we can use unique_ptr instead of | ||
| 360 | - shared_ptr as a replacement for PointerHolder. For performance | ||
| 361 | - critical cases, we could potentially have a real pointer and a | ||
| 362 | - shared pointer where the shared pointer's job is to clean up but we | ||
| 363 | - use the real pointer for regular access. | 423 | +* See PointerHolder to std::shared_ptr above. |
| 364 | * See where anonymous namespaces can be used to keep things private to | 424 | * See where anonymous namespaces can be used to keep things private to |
| 365 | a source file. Search for `(class|struct)` in **/*.cc. | 425 | a source file. Search for `(class|struct)` in **/*.cc. |
| 366 | * See if we can use constructor delegation instead of init() in | 426 | * See if we can use constructor delegation instead of init() in |