Opened 5 years ago

Last modified 4 years ago

#1062 reviewing change

Add noncopyable base class and use it

Reported by: sergz Assignee: sergz
Priority: P4 Milestone:
Module: Adblock-Plus-for-Internet-Explorer Keywords:
Cc: Blocked By:
Blocking: Platform: Internet Explorer
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/5769493129199616/

Description

It's convenient, because it shows the problems at the compile time and describes the copy interface of the class (apparently, it's not copyable).

What to do

Just copy the impl from boost, include into shared (utils) project and go through all classes in the project and check whether they are supposed to be copyable or not.

Change History (6)

comment:1 Changed 5 years ago by sergz

  • Component changed from Unknown to Adblock-Plus-for-Internet-Explorer
  • Priority changed from Unknown to P4

comment:2 Changed 5 years ago by eric@…

It would be better to use the "= deleted" syntax in C++11. The Boost solution worked adequately for its time, but doesn't play with POD support in C++11. In addition, I've already come across cases where I want to disable copying but not moving (e.g. certain kinds of factories), and the noncopyable base class doesn't work for this. I'd need to delete the copy functions with the new syntax anyway.

Deleted functions are not supported in VS 2012, but is in VS 2103. See https://issues.adblockplus.org/ticket/1087

comment:3 Changed 5 years ago by eric@…

  • Summary changed from Add nonecopyable base class and use it to Add noncopyable base class and use it

comment:4 Changed 5 years ago by sergz

I'm not sure that we should not use noncopyable. It's very descriptive, less error prone and I am pretty sure we don't use it for POD which are passed to C.
What are the benefits from using " = deleted" in comparison with noncopyable?

Could you describe the problem when noncopyable breaks the movability?

comment:5 Changed 4 years ago by oleksandr

  • Ready set
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:6 Changed 4 years ago by oleksandr

  • Owner set to sergz
Note: See TracTickets for help on using tickets.