R.5 and who decides necessity? (Prefer scoped objects, don't heap-allocate unnecessarily)

[kwbBaker]

So it's kind of a best-practice issue: should you always prefer local (stack-allocated) objects over smart pointers and putting them on the heap as suggested by R.5, or are very large objects better placed on the heap to avoid huge stack requirements from a function? I didn't find any discussion about this guideline. Might you think of other situations that could make heap-allocation seem "necessary", that this rule might not consider?

The reason I ask...

In cleaning up some code, I ran into numerous places where unusually large objects were being created on the stack as variables local to functions (in case you care, they were usually DAO objects -- this is some very old MFC code), and the compiler was suggesting I should consider putting them on the heap. Presumably, it can be very bad to have functions suck up huge chunks of the stack. But that warning was coming from Visual Studio's compiler, not the guidelines. Okay, I can do that. Most of these were something like "TheBigType TheVar;". I don't remember all the sizes the warning was reporting as what the function would require of the stack, but it was usually in the tens of kilobytes. I'm not sure what the threshold is for triggering that warning.

So I converted these over to using something like "auto pTheVar = std::make_unique<TheBigType>()" -- usually accompanied by changing the uses of TheVar.field to pTheVar->field or the passing of &TheVar to a called function to instead pass pTheVar.get(). After making such changes, then I got the guideline R.5 warning triggered by my call to make_unique: "Move, copy, reassign or reset a local smart pointer" (even though it was actually an initializer, not any of the things in that list). The description for this warning points to guidelines R.5, "R.5: Prefer scoped objects, don't heap-allocate unnecessarily".

Indeed, there doesn't seem to be much leeway on that, as if it's always preferable to put the object onto the stack when you can, instead of putting it on the heap. So these two "best practices" warning seem to be at odds with one another. But was Visual Studio's compiler in error to call out this case a violating R.5 for an initializer? Based on the description, it seems consistent with R.5. Or are these truly conflicting ideas, here (the other being the idea that you shouldn't have a function use "too much" stack space)?

[kwbBaker]

Actually, I should have said I was most often using an initializer, not a copy/move constructor, as in "auto pTheVar{ std::make_unique<TheBigType>() };".

[jwakely]

That still uses the move constructor.

[jwakely]

I don't know what you mean by "in error to call out this case a violating R.5 for an initializer?" As I said above, "using an initializer" doesn't mean anything here, this also uses an initializer:

auto pTheVar = std::make_unique<TheBigType>();

Maybe you mean using list-initialization syntax, but that's irrelevant, it doesn't change anything about the logic of the code.

What the guideline suggests, and Visual Studio seems to be following, is that a local smart pointer where you don't explicitly do something with the heap object (e.g. transfer ownership of it elsewhere, out of the function) might as well be on the stack and not on the heap. But I agree that large objects are a valid reason to avoid the stack, and so maybe the guideline should have an exception for very large objects (for some value of large - presumably Visual Studio could use the same threshold it uses for warning you to put them on the heap).

I think you could silence the complaint by adding a redundant pTheVar.reset(); before returning from the function (with a comment saying why it's done).

[kwbBaker]

[where you quoted me, there was a typo (mine): the word "a" should have been "as"] I just didn't know that no matter what I did there, it would use a "Move".

I didn't think your suggestion could work since the actual message from the compiler is

Move, copy, reassign or reset a local smart pointer 'pTheVar'

But I tried it (at the end of the function, after the last use of that smart pointer), and indeed, the warning disappeared. I wouldn't have guessed that could help, given the message. But thanks for the workaround anyway!

[hsutter]

Editors call: Thanks! Done.

[jwakely]

I wouldn't have guessed that could help, given the message. But thanks for the workaround anyway!

The way I interpret the message is that is's saying that you should do one of those things to make the compiler think you're "using" the smart ptr in some way. Since reset is one of the things it says to do, I guessed that calling reset() would make the compiler happy.