FYI: Move-only class + windows C++ compiler + ternary operator

[vabr]

Hi all,

The following describes a strange issue of the compiler on win_rel try bot.

Albert Wong was very kind and took a look at this, and said he was not sure whether this is a compiler bug or something which can be fixed in our code (base/move.h or scoped_ptr). Therefore I have not filed a bug and I'm only posting a work-around here just in case anybody hits the same problem later. If you think this is something we can and should fix, feel free to let me know and I'll file the bug.

The following code seems OK to me and (with details in) compiles all right on linux and mac try-bots:

scoped_ptr<const DictionaryValue> GetDictionaryFromJson(/*...*/) {

  bool success;

  // ...

  scoped_ptr<const DictionaryValue> dictionary(/*...*/);

  return success ? dictionary.Pass() : scoped_ptr<const DictionaryValue>();

}

However, the win_rel bot complained (search for "C2248" on that page) about the return statement, pointing to the line where scoped_ptr employs MOVE_ONLY_TYPE_FOR_CPP_03, and saying that "'scoped_ptr<C>::scoped_ptr' : cannot access private member declared in class 'scoped_ptr<C>'". Interestingly after I expanded the ternary operator to a full if-else statement, the compiler accepted it.

Vaclav

[Mattias Nissler]

For posterity, here is the full error message (the log will become unavailable at some point):

E:\b\build\slave\win\build\src\chrome\browser\extensions\api\web_request\web_request_api_unittest.cc(96) :error C2248: 'scoped_ptr<C>::scoped_ptr' : cannot access private member declared in class 'scoped_ptr<C>'

        with
        [
            C=const base::DictionaryValue
        ]
        ..\base/memory/scoped_ptr.h(134) : see declaration of 'scoped_ptr<C>::scoped_ptr'
        with
        [
            C=const base::DictionaryValue
        ]

Vaclav

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Jochen Eisinger]

The return value of a ternary operator is the type of the second expressions, which is the (private) class scoped_ptr<const::DictionaryValue>::RValue. The third expression is of type scoped_ptr<const::DictionaryValue> which you cannot cast to ...::RValue

I guess that's what the error message is trying to tell you

-jochen

[vabr]

Thanks, Jochen,

On Friday, August 3, 2012 11:42:43 AM UTC+2, Jochen Eisinger wrote:

The return value of a ternary operator is the type of the second expressions, which is the (private) class scoped_ptr<const::DictionaryValue>::RValue. The third expression is of type scoped_ptr<const::DictionaryValue> which you cannot cast to ...::RValue

I guess that's what the error message is trying to tell you

Your explanation makes sense (unlike the error message from the compiler).

But I cannot help wondering why did it work on the other compilers...

Vaclav

 

-jochen

On Fri, Aug 3, 2012 at 10:46 AM, Mattias Nissler wrote:

[Victor Khimenko]

On Fri, Aug 3, 2012 at 1:50 PM, vabr <va...@chromium.org> wrote:

Thanks, Jochen,

On Friday, August 3, 2012 11:42:43 AM UTC+2, Jochen Eisinger wrote:

The return value of a ternary operator is the type of the second expressions, which is the (private) class scoped_ptr<const::DictionaryValue>::RValue. The third expression is of type scoped_ptr<const::DictionaryValue> which you cannot cast to ...::RValue

I guess that's what the error message is trying to tell you

Your explanation makes sense (unlike the error message from the compiler).

But I cannot help wondering why did it work on the other compilers...

Because they correctly implement the C++ standard?

Type of the ternary operator is NOT the type of the second expression. Instead if types are different then compiler is supposed to try to convert T1 to T2 and T2 to T1. If one is Ok and another is not then this conversion is chosen, if both are valid then program is malformed. Here scoped_ptr<const::DictionaryValue> will be converted to scoped_ptr<const::DictionaryValue> because scoped_ptr<const::DictionaryValue>::RValue can not be converted to scoped_ptr<const::DictionaryValue> as was pointed above. Was defined this way from the day one (C++98).

If Jochen's explanation is correct then it should be easy to check: swap arguments of ternary operand around and see if it'll compile or not.

Vaclav

 

-jochen

On Fri, Aug 3, 2012 at 10:46 AM, Mattias Nissler wrote:

On Fri, Aug 3, 2012 at 10:13 AM, vabr wrote:

Hi all,

The following describes a strange issue of the compiler on win_rel try bot.

Albert Wong was very kind and took a look at this, and said he was not sure whether this is a compiler bug or something which can be fixed in our code (base/move.h or scoped_ptr). Therefore I have not filed a bug and I'm only posting a work-around here just in case anybody hits the same problem later. If you think this is something we can and should fix, feel free to let me know and I'll file the bug.

The following code seems OK to me and (with details in) compiles all right on linux and mac try-bots:

scoped_ptr<const DictionaryValue> GetDictionaryFromJson(/*...*/) {

  bool success;

  // ...

  scoped_ptr<const DictionaryValue> dictionary(/*...*/);

  return success ? dictionary.Pass() : scoped_ptr<const DictionaryValue>();

}

However, the win_rel bot complained (search for "C2248" on that page) about the return statement, pointing to the line where scoped_ptr employs MOVE_ONLY_TYPE_FOR_CPP_03, and saying that "'scoped_ptr<C>::scoped_ptr' : cannot access private member declared in class 'scoped_ptr<C>'". Interestingly after I expanded the ternary operator to a full if-else statement, the compiler accepted it.

For posterity, here is the full error message (the log will become unavailable at some point):

E:\b\build\slave\win\build\src\chrome\browser\extensions\api\web_request\web_request_api_unittest.cc(96) :error C2248: 'scoped_ptr<C>::scoped_ptr' : cannot access private member declared in class 'scoped_ptr<C>'

        with
        [
            C=const base::DictionaryValue
        ]
        ..\base/memory/scoped_ptr.h(134) : see declaration of 'scoped_ptr<C>::scoped_ptr'
        with
        [
            C=const base::DictionaryValue

        ]

Vaclav

[vabr]

Thanks, that's interesting.

On Friday, August 3, 2012 12:09:39 PM UTC+2, khim wrote:

On Fri, Aug 3, 2012 at 1:50 PM, vabr wrote:

Thanks, Jochen,

On Friday, August 3, 2012 11:42:43 AM UTC+2, Jochen Eisinger wrote:

The return value of a ternary operator is the type of the second expressions, which is the (private) class scoped_ptr<const::DictionaryValue>::RValue. The third expression is of type scoped_ptr<const::DictionaryValue> which you cannot cast to ...::RValue

I guess that's what the error message is trying to tell you

Your explanation makes sense (unlike the error message from the compiler).

But I cannot help wondering why did it work on the other compilers...

Because they correctly implement the C++ standard?

Type of the ternary operator is NOT the type of the second expression. Instead if types are different then compiler is supposed to try to convert T1 to T2 and T2 to T1. If one is Ok and another is not then this conversion is chosen, if both are valid then program is malformed. Here scoped_ptr<const::DictionaryValue> will be converted to scoped_ptr<const::DictionaryValue> because scoped_ptr<const::DictionaryValue>::RValue can not be converted to scoped_ptr<const::DictionaryValue> as was pointed above. Was defined this way from the day one (C++98).

But might there be also a problem with the type of return value being scoped_ptr<const::DictionaryValue>? If the ternary expression ends up being of type RValue, then it should not be possible to convert it to scoped_ptr<const::DictionaryValue>, right? Or does the RValue propagate further, until the point where the result of GetDictionaryFromJson is used as as a right-hand value?

[Victor Khimenko]

On Fri, Aug 3, 2012 at 2:30 PM, vabr <va...@chromium.org> wrote:

Thanks, that's interesting.

On Friday, August 3, 2012 12:09:39 PM UTC+2, khim wrote:

On Fri, Aug 3, 2012 at 1:50 PM, vabr wrote:

Thanks, Jochen,

On Friday, August 3, 2012 11:42:43 AM UTC+2, Jochen Eisinger wrote:

The return value of a ternary operator is the type of the second expressions, which is the (private) class scoped_ptr<const::DictionaryValue>::RValue. The third expression is of type scoped_ptr<const::DictionaryValue> which you cannot cast to ...::RValue

I guess that's what the error message is trying to tell you

Your explanation makes sense (unlike the error message from the compiler).

But I cannot help wondering why did it work on the other compilers...

Because they correctly implement the C++ standard?

Type of the ternary operator is NOT the type of the second expression. Instead if types are different then compiler is supposed to try to convert T1 to T2 and T2 to T1. If one is Ok and another is not then this conversion is chosen, if both are valid then program is malformed. Here scoped_ptr<const::DictionaryValue> will be converted to scoped_ptr<const::DictionaryValue> because scoped_ptr<const::DictionaryValue>::RValue can not be converted to scoped_ptr<const::DictionaryValue> as was pointed above. Was defined this way from the day one (C++98).

But might there be also a problem with the type of return value being scoped_ptr<const::DictionaryValue>? If the ternary expression ends up being of type RValue, then it should not be possible to convert it to scoped_ptr<const::DictionaryValue>, right?

Right. If you can convert both ways then program is malformed and compiler should not accept it.

 

Or does the RValue propagate further, until the point where the result of GetDictionaryFromJson is used as as a right-hand value?

 

No. The only place where C++ does such an awful thing and determines type of something using the destination type if when you take an address of the overloaded function. In all other places arguments should determine the type of outcome. In particular ternary operator picks either T1 or T2, if it can not determine which one is "correct" it should detect some kind of error.

 

If Jochen's explanation is correct then it should be easy to check: swap arguments of ternary operand around and see if it'll compile or not.

Vaclav

 

-jochen

On Fri, Aug 3, 2012 at 10:46 AM, Mattias Nissler wrote:

On Fri, Aug 3, 2012 at 10:13 AM, vabr wrote:

Hi all,

The following describes a strange issue of the compiler on win_rel try bot.

Albert Wong was very kind and took a look at this, and said he was not sure whether this is a compiler bug or something which can be fixed in our code (base/move.h or scoped_ptr). Therefore I have not filed a bug and I'm only posting a work-around here just in case anybody hits the same problem later. If you think this is something we can and should fix, feel free to let me know and I'll file the bug.

The following code seems OK to me and (with details in) compiles all right on linux and mac try-bots:

scoped_ptr<const DictionaryValue> GetDictionaryFromJson(/*...*/) {

  bool success;

  // ...

  scoped_ptr<const DictionaryValue> dictionary(/*...*/);

  return success ? dictionary.Pass() : scoped_ptr<const DictionaryValue>();

}

However, the win_rel bot complained (search for "C2248" on that page) about the return statement, pointing to the line where scoped_ptr employs MOVE_ONLY_TYPE_FOR_CPP_03, and saying that "'scoped_ptr<C>::scoped_ptr' : cannot access private member declared in class 'scoped_ptr<C>'". Interestingly after I expanded the ternary operator to a full if-else statement, the compiler accepted it.

For posterity, here is the full error message (the log will become unavailable at some point):

E:\b\build\slave\win\build\src\chrome\browser\extensions\api\web_request\web_request_api_unittest.cc(96) :error C2248: 'scoped_ptr<C>::scoped_ptr' : cannot access private member declared in class 'scoped_ptr<C>'

        with
        [
            C=const base::DictionaryValue
        ]
        ..\base/memory/scoped_ptr.h(134) : see declaration of 'scoped_ptr<C>::scoped_ptr'
        with
        [
            C=const base::DictionaryValue

        ]

Vaclav