Opened on 07/16/2014 at 01:23:37 PM

Last modified on 12/11/2014 at 09:47:19 AM

#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.

Attachments (0)

Change History (6)

comment:1 Changed on 07/16/2014 at 01:31:24 PM by sergz

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

comment:2 Changed on 07/18/2014 at 07:29:49 PM by eric@adblockplus.org

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 on 07/18/2014 at 07:30:30 PM by eric@adblockplus.org

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

comment:4 Changed on 07/21/2014 at 11:34:49 AM 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 on 12/11/2014 at 09:46:55 AM by oleksandr

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

comment:6 Changed on 12/11/2014 at 09:47:19 AM by oleksandr

  • Owner set to sergz

Add Comment

Modify Ticket

Change Properties
Action
as reviewing .
as The resolution will be set. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from sergz.
 
Note: See TracTickets for help on using tickets.