BIND 10 #2144: add nonassignable super class

BIND 10 Development do-not-reply at isc.org
Tue Mar 26 21:36:10 UTC 2013


#2144: add nonassignable super class
-------------------------------------+-------------------------------------
            Reporter:  fdupont       |                        Owner:
                Type:  enhancement   |  jinmei
            Priority:  medium        |                       Status:
           Component:  Unclassified  |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130402
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  2             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:12 fdupont]:

 > > '''nonassignable.h and noncopyable.h'''
 > >
 > > - First off: do we need our own noncopyable, if it's just a copy of
 > >   Boost?
 >
 > => a copy

 I don't understand this.  My question is why not just keep using
 boost::noncopyable.

 > > - Please add documentation: brief description of what it is, the
 > >   purpose of these classes, in which case we should use which, and
 > >   why, etc.
 >
 > => do we need more than Boost doc? BTW as far as I can understand of
 Visual Studio explanations the use is for classes with const members:

 If we use the Boost version for noncopyable, obviously we don't need
 to write anything ourselves for it (see the previous point).  And, for
 nonassignable, there's no Boost implementation, much less
 documentation, so we need to write it ourselves anyway.  And, how we
 use them in our own code is probably specific to our own situation and
 worth documenting.

 > > - Shouldn't we also copy Boost copyright?
 > > {{{#!cpp
 > > /// From boost/noncopyable.hpp
 > > }}}
 > >   even though the definition is quite trivial.
 >
 > => I don't know the exact policy but IMHO we should.

 I've added it to nonassignable.  For noncopyable, I still believe it's
 better to just use the Boost version than maintaining our own copy.

 > > - what's the intent of the postfix underscore in `noncopyable_`?
 > > {{{#!cpp
 > > namespace noncopyable_ {
 > > }}}
 > >   and the typedef?
 > > {{{#!cpp
 > > typedef noncopyable_::noncopyable noncopyable;
 > > }}}
 >
 > => it is from Boost so ask a Boost person...

 Actually the Boost implementation clarifies that:
 {{{
 namespace noncopyable_  // protection from unintended ADL
 }}}
 I'm not really sure if the class definition could cause any ADL
 related troubles, but at least I can see some possibility.  I've added
 the same single comment to nonassignable.h.  Again, for noncopyable, I
 believe it's better to just use the Boost version than maintaining our
 own copy so I didn't touch it.

 > > '''labelsequence'''
 > >
 > > - The restriction of the assignment operator seems quite counter
 > >   intuitive and inconvenient.  What's the rationale of it?  At the
 > >   very least this should be documented.
 >
 > => note I agree with Microsoft assignment operator and const members
 > are not really compatible...

 We don't use operator= except in some part of labelsequence_unittest,
 so I suggest making it assignable at least for now and updating the
 unittest so it doesn't use the operator.  Making `LabelSequence`
 assignable may be useful, but it will increase the risk of
 unintentional data sharing, so unless we really need it it's probably
 safer to prohibit it explicitly.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2144#comment:14>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list