Does this pattern work in C++?

Question

I'm running PREfast static code analysis on my our projects, and it is giving me C6001 'using uninitialized memory' errors for this pattern:

// AutoSelectGDIObject is a class
if (AutoSelectGDIObject & select_image = AutoSelectGDIObject(hDCImgSource, hBmp))
{
   // use hDCImgSource knowing that hBmp is selected into it
}
// now we are guaranteed that hDCImgSource has had hBmp removed and its previous bmp re-selected into itself

The trick I'm trying to exploit is to allow the scoping of select_image to just the if-expression (i.e. if (condition) { expression-block = lifetime of condition variable }.

VS has happily compiled (and presumably run this) for quite some time. I haven't single stepped code like this in a long time, but as far as I can tell, it only enters the if block if select_image's operator bool() returns true, and it destroys the instance of select_image upon exiting that if block.

Is PREfast wrong? Or is there something subtle here that makes my above code and assumptions incorrect?

`AutoSelectGDIObejct` is a class. I'm using a reference to get around the fact that C++ won't allow me to just use `if (AutoSelectGDIObject select_image(...))` <- doesn't compile :( — Mordachai, Apr 17 '13 at 14:34
It shouldn't. Here is the reason: http://stackoverflow.com/questions/15580903/scope-of-an-inside-parenthesis-declared-object — fatihk, Apr 17 '13 at 14:35
@Mordachai: Could you please edit your question and specify that `AutoSelectGDIObject` is a class type, and not a function pointer as some may assume? — yzt, Apr 17 '13 at 14:38

Andy Prowl · Accepted Answer · 2013-04-17T14:47:41.533

4

Is PREfast wrong? Or is there something subtle here that makes my above code and assumptions incorrect?

That code is invalid C++, but VC compiles it because it allows, as a documented extension, binding lvalue references to non const-qualified types to temporaries.

This is a silly extension, in my opinion. According to the C++ Standard, temporaries can only be bound lvalue references to const or to rvalue references (and in this case, their lifetime is extended beyond the end of the full-expression that creates them).

Therefore, your if statement should be:

if (AutoSelectGDIObject const& select_image = 
//                      ^^^^^
                               AutoSelectGDIObject(hDCImgSource, hBmp))

See, for instance, this live example.

Notice, however, that you should not need to use references at all here. In other words, the following if statement is also valid, and expresses your intention better than creating a temporary bound to an lvalue reference to const:

if (AutoSelectGDIObject select_image = AutoSelectGDIObject(hDCImgSource, hBmp))

See, for instance, this live example. Besides, the above would also allow you to modify select_image, while the reference to const will not.

edited Apr 17 '13 at 14:47

answered Apr 17 '13 at 14:35

Andy Prowl

124,023
23
387
451

AutoSelectGDIObject seems to be the name of a class here. – mfontanini Apr 17 '13 at 14:37
Okay - good to know. And with the change to const&, you're saying that the code is valid (i.e. that the if-condition will eval to true IFF select_image.operator bool() is true?) – Mordachai Apr 17 '13 at 14:39
@Mordachai: It is valid, yes. See, for instance, [this example](http://ideone.com/MU2q6w). – Andy Prowl Apr 17 '13 at 14:40
@AndyProwl I choose the const & because it guarantees that no copies are made. The AutoSelectGDIObject is non-copyable. It could be made copyable, but that's just machinery in order to satisfy a certain C++ style, rather than actually more expressive or even more correct (under the hood, it would have to avoid actually don't things in the case of a copy - it would be a hand-off operation, which in general doesn't make sense for such a class). C++11 makes it possible to do that correctly, but again, for what gain? – Mordachai Apr 17 '13 at 17:27
@Mordachai : `if (AutoSelectGDIObject&& select_image = AutoSelectGDIObject(hDCImgSource, hBmp))` would be valid as well, but it's a moot point, as `if (AutoSelectGDIObject select_image = AutoSelectGDIObject(hDCImgSource, hBmp))` would perform a move, not a copy (and that move is almost guaranteed to be elided anyway). – ildjarn May 03 '13 at 21:38

Does this pattern work in C++?

1 Answers1