Clean-up inline members of nested classes (base/)

base/values.h

Index: base/values.h
diff --git a/base/values.h b/base/values.h
index c5834070352156dca888de3529778a673c879a1f..87d195d8a6597d4c2226f6f868536a5fd9fee166 100644
--- a/base/values.h
+++ b/base/values.h
@@ -342,7 +342,8 @@ class BASE_EXPORT DictionaryValue : public Value {
   class key_iterator
       : private std::iterator<std::input_iterator_tag, const std::string> {

[jar (doing other things), 2012/08/09 16:30:27]

Any idea why this is using private inheritance (violating style guide)?  Do you
know how this functionality is used at all in the class?

[hans, 2012/08/09 18:28:48]

No idea, I haven't really looked at the file itself, this is just a mechanical
change.

    public:
-    explicit key_iterator(ValueMap::const_iterator itr) { itr_ = itr; }
+    explicit key_iterator(ValueMap::const_iterator itr);
+    key_iterator(const key_iterator& rhs);

[jar (doing other things), 2012/08/09 16:30:27]

a) Why did you need to add this, if this CL is all about warnings?

b) Please add the explicit keyword.

[hans, 2012/08/09 18:28:48]

Because the plugin warns that this class needs an out-of-line copy constructor.

If we don't add one, the compiler will synthesize a default one for us, which is
inline, and the style guide tries to limit the amount of complex inline
functions in header files.
I don't think we do that for copy constructors.

[jar (doing other things), 2012/08/09 19:27:16]

You are correct about explicit. Thanks!

I think the style guide asks for a comment to that effect (noting why it is an
exception).




On 2012/08/09 18:28:48, hans wrote:

[hans, 2012/08/09 19:31:05]

Comment added.

     key_iterator operator++() {
       ++itr_;
       return *this;
@@ -362,8 +363,7 @@ class BASE_EXPORT DictionaryValue : public Value {
   // dictionary.  It can't be used to modify the dictionary.
   class Iterator {
    public:
-    explicit Iterator(const DictionaryValue& target)
-        : target_(target), it_(target.dictionary_.begin()) {}
+    explicit Iterator(const DictionaryValue& target);
 
     bool HasNext() const { return it_ != target_.dictionary_.end(); }
     void Advance() { ++it_; }