Pull to refresh

Comments 5

Большое спасибо, Алексей!

Хотел бы обсудить последний пункт последнего раздела «Never dereference or call a method of non-local shared_ptr, because this puts object outside of shared_ptr control (instead, first make a local copy of shared_ptr)» в сочетании с "// global (static or heap) or aliased local" комментарием в иллюстрирующем коде.

Далее в иллюстрирующем коде создаётся стабилизирующая локальная копия shared_ptr. Мотив понятен — действия внутри f могут привести к удалению widget-а. Таким образом, этот код иллюстрирует определённый «параноидально-защитный» паттерн программирования: в месте использования определённого типа ссылки (shared_ptr) знай, что безопаснее защититься от reentrancy pitfall.

Давайте теперь, взяв на вооружение этот совет (т.е., встав на сторону данного «параноидально-защитного паттерна»), рассмотрим первые две рекомендации раздела:

  • Do not pass shared_ptr (or other countref alternatives) by value, if you do not need to count references (reduces performance due to atomic operations with counter) — prefer passing objects by * or & as usual.
  • Do not pass shared_ptr (or other countref alternatives) by reference or const reference, if you do not need to count references — prefer passing objects by * or & as usual.

Давайте попытаемся совместить логику этих рекомендаций с «параноидально-защитным» паттерном. Caller имеет на руках shared_ptr на widget, который нужно передать в callee (под названием «use» — callee не интересуют манипуляции с ownership widget-а, он просто что-то делает с самим widget-ом). Рекомендовано передать ссылку на widget (то есть в сигнатуре use тип параметра — widget&). Получается, чтобы соблюсти «параноидально-защитный» паттерн, caller перед вызовом use должен удостовериться, что передаваемая ссылка подкреплена локальным инстансом shared_ptr. То есть, если shared_ptr на руках, например, member variable под именем m_p, надо делать так:

shared_ptr<widget> pStabilized = m_p;

Эту манипуляцию можно было бы выразить короче, если бы (в разрез с первой рекомендацией раздела) параметр use был бы shared_ptr (переданный по значению):


То есть, в случае принятия обоих рекомендаций раздела (первой и последней) caller должен платить за комфорт callee и создавать стабилизирующие локальные копии не-локальных shared_ptr, так?
Hi. Could you please use english language in comments, so that both the post and the discussion are useful to international audience. So, the question is: should we pass shared_ptr by value or create a local copy and dereference it. I added some links with the source of this recommendation so that you can get more detailed information about it.

Passing shared_ptr by value obviously looks more simple and «comfortable» as you say in this simple example, but it also has downsides that I can think of, which may be important or not important in your code:
— you are passing to the function more than it needs (if it just needs a reference) and thus we incapsulate additional logic into that function, making it less general and less eligible for reuse. This function cannot be used for a non-shared_ptr
— you will have to make copy of shared_ptr with every call of this function if you will need to call it multiple times
— you will have to make copy of shared_ptr with every call of other functions in this block if you follow this approach

Also, please note that recommendation is to use auto instead of shared_ptr — you can find out why this is important by following the same link to youtube video.
Regarding using English — sure why not (I cannot update the initial comment but I'll retell the core concern in brief below). Thanks for the links you provided in the post, I will trace details when I have a better chance (over the weekend, maybe). Thank you for listing arguments for not passing smart pointer arguments in favor of passing & references (stating explicitly that the contract guarantees that the reference is not null always) or * pointers (stating explicitly that the contract allows nullptr reference on some occasions so the callee has to handle that possibility in a meaningful way). I've read Core Guidelines and I agree, but a consideration linked to the last recommendation (see below) leaves a residue so to speak.

Regarding my core concern on the last section of your post: the last recommendation of that section kind of introduces a complication twist to «prefer using &, * and not smart pointer argument types» rule. Let' get back to my example: I am implementing a class that has a member variable shared_ptr<widget> m_p. It is guaranteed to be not null at a moment and I need to call an external function named «use» that takes widget&. Being a thorough defensive programmer (as the last recommendation teaches me) I have to do the following (now switching to auto):

auto pStabilized = m_p;

— because from a formal point of view I have no control over what other functions «use» will call. Note that in case of 5 reference arguments of «use» the caller will have 6 lines of code introducing 5 local variables that are used in one line of code — the «use» call. Well, it takes an effort to be a thorough defensive programmer (and never mind the impact on code readability / maintainability). Now imagine that I am not only doing that in my code but also insist on this pattern while doing code reviews…

Do you agree that this is the right conclusion when all recommendations of the last section are taken into account? If you have a better idea on how to reconcile all recommendations from the last section, please share! I'll do some experiments most likely, but not earlier than over the next weekend.
As I have said, downsides can be unimportant in your code. Decision has to be made for each particular part of code where you use such an approach.
In my current projects I am inclined to not follow the last recommendation ("Never dereference or call a method of non-local shared_ptr, because this puts object outside of shared_ptr control (instead, first make a local copy of shared_ptr).").

That recommendation does not blend well with preceding ones that are not that over-protective or super-defensive. Let it crash, so to speak.

Your 2-part article is very good, I'll be glad to see more!
Only those users with full accounts are able to leave comments. Log in, please.