3

I wrote a scope guard which resets a value when the scope exits:

template <class T>
struct ResetGuard
{
    T old_value;
    T& obj_to_reset;
    ResetGuard(T& obj_to_reset, const T& new_value) :
        old_value(obj_to_reset),
        obj_to_reset(obj_to_reset)
    {
        obj_to_reset = new_value;
    }

    ~ResetGuard() { obj_to_reset = old_value; }
};

When this scope guard is returned from a function, is there any way to prevent the immediate destruction of the scope guard if it wasn't saved?

For example:

int GLOBAL_VALUE = 0;
ResetGuard<int> temporarily_set_global_value(int new_val) {
    return { GLOBAL_VALUE, new_val }; //updates the global variable
}
void foo() {
    //Ideally, someone calling this function
    //Wouldn't have to save the returned value to a local variable
    temporarily_set_global_value(15);
    std::cout << "GLOBAL_VALUE is " << GLOBAL_VALUE << std::endl;
}

The way it's written now, anyone calling one of the functions would have to remember to always save the ResetGuard to a local variable, otherwise it would immediately reset the value.

Some context around what I'm trying to do

I'm writing a library to format and manipulate strings. I have one global variable controlling how floating point numbers are formatted. I know that global variables are typically a terrible idea, but please bear with me.

I made the decision to use a global variable carefully. The alternative to using a global variable would be to pass around the object containing the formatting specification. This option ultimately proved infeasible: my library is designed to work with any objects that provide a implicit conversion to std::string. There's no way to pass formatting options (or any parameters, really) to an implicit conversion function. Hence, I had to resort to using a global variable.

JVApen
  • 11,008
  • 5
  • 31
  • 67
Antonio Perez
  • 463
  • 3
  • 10
  • 3
    How about implementing the move constructor correctly? (PS: don't forget the rule of 5) – JVApen Jul 01 '18 at 06:50
  • 2
    (That all sounds horrible.) Why are you trying to avoid a local variable? That's the common idiom for this sort of thing. – Mat Jul 01 '18 at 06:50
  • 1
    This is a minimal working example. I'm trying to avoid a local variable because I want to make the code as bullet proof as possible: I don't want something to fail because someone forgot to save the scope guard to a local variable. – Antonio Perez Jul 01 '18 at 06:59
  • 1
    Why don't you just mark it as [`struct [[nodiscard]] ResetGuard`](https://en.cppreference.com/w/cpp/language/attributes/nodiscard)? – melpomene Jul 01 '18 at 07:08
  • 1
    Sounds like an [xy problem](https://meta.stackexchange.com/a/66378)... – Aconcagua Jul 01 '18 at 07:17
  • What do you intend when `temporarily_set_global_value` would call another function, say `bar` which also sets the global variable? Should it recursively restore the value as if you are building a stack of global states? – André Jul 01 '18 at 07:17
  • @melpomene Posted a proper answer, live demo to be added. Thank you for picking me up on this, the first one was terrible. – Paul Sanders Jul 01 '18 at 08:32
  • Make your class *movable*. – n. m. could be an AI Jul 01 '18 at 08:54
  • @n.m Noted, I'll add that to my answer when I get to posting some code. – Paul Sanders Jul 01 '18 at 09:02

3 Answers3

2

Before answering your question, I like to provide the correct way of solving this problem in C++.

template <class T>
struct [[nodiscard]] ResetGuard
{
    T old_value;
    T& obj_to_reset;
    bool enabled{true};

    ResetGuard(T& obj_to_reset, const T& new_value) :
       old_value(obj_to_reset),
       obj_to_reset(obj_to_reset)
    {
       obj_to_reset = new_value;
    }

    ResetGuard(ResetGuard &&rhs)
       : old_value(rhs.old_value)
       , obj_to_reset(rhs.obj_to_reset)
    {
        rhs.enabled = false;
    }
    ~ResetGuard()
    {
        if (enabled)
            obj_to_reset = old_value;
    }
    ResetGuard(const ResetGuard &) = delete;
    ResetGuard &operator=(const ResetGuard &) = delete;
    ResetGuard &operator=(ResetGuard &&) = delete;  
};

void foo() {
    auto guard = temporarily_set_global_value(15);
    std::cout << "GLOBAL_VALUE is " << GLOBAL_VALUE << std::endl;
}

The above code contains several interesting elements:

  • [[nodiscard]] prevent creating temporaries without creating a variable to ensure scope
  • The member enabled: Prevent the Dtor of a temporary to have a side effect
  • Move constructor: The move constructor allows moving the ResetGuard into a different scope with the right handling. In this case, disabling the old ResetGuard

As an extra note, I like to point attention to a C++17 extension (previously allowed optimization), which is called Guaranteed Copy/Move Elision. This will ensure that in practice no extra temporary instances will exist.

Back to your question: Is there any way to extend the lifetime of a temporary object in C++?

Yes, thanks to N0345 (Proposal from 1993). This proposal allows extension of a temporary by capturing it by const reference.

const auto &guard = temporarily_set_global_value(15);

However, it is unclear to me how many instances you will have in total. However, if you use the solution with the move constructor, this is no longer an issue. Further more, when you use compiler optimizations, this function could be inlined when implemented in the header. That could eliminate all copies.

JVApen
  • 11,008
  • 5
  • 31
  • 67
  • I don't think there will be any copies anyway, even if you simply say `auto guard = temporarily_set_global_value(15);`. When you do that, the object is actually allocated at the calling site and `temporarily_set_global_value` will construct it in place (via a hidden pointer passed to it), see: [here](https://stackoverflow.com/a/51104498/5743288) (bit of a long-winded post, sorry). It's a non-issue these days, basically. – Paul Sanders Jul 01 '18 at 18:16
  • @paul: im familiar with amd64, where that's indeed the case. Wasn't sure about other configurations – JVApen Jul 01 '18 at 18:21
  • @JV I think it's actually required now, see https://en.cppreference.com/w/cpp/language/copy_elision. – Paul Sanders Jul 01 '18 at 18:37
  • That's only when you compile with c++17 – JVApen Jul 01 '18 at 18:38
  • Yes, you're right. But even if you go back a ways, it still seems to actually work that way, see https://wandbox.org/permlink/rUbqZ1lWawb2aWXC, so I think this is just a formsalisation of what compilers have been doing for quite some time now. – Paul Sanders Jul 01 '18 at 19:46
  • Missing a `rhs.` in the move constructor. `ResetGuard(ResetGuard &&rhs) : old_value(rhs.old_value), obj_to_reset(obj_to_reset)` should be `ResetGuard(ResetGuard &&rhs) : old_value(rhs.old_value), obj_to_reset(rhs.obj_to_reset)` Otherwise it just assigns `obj_to_reset` to itself, and you will have a bad time when the destructor is called and it writes to a random memory address ... ;) – Steven Spark Jul 18 '23 at 20:53
  • Thanks, fixed it. This site would really benefit from showing compiler warnings – JVApen Jul 19 '23 at 04:51
1

Is there any way to extend the lifetime of a temporary object in C++?

Only one way, assign it to a variable (possibly a reference). If you don't want to burden users of the library, you can hide the details behind a macro. While it's true that the uses of macros become few and far between, this is something you can only do with a macro. For instance, here's how you'd do it with a sprinkle of GCC extensions:

#define CONCAT(a, b) a##b
#define SCOPED_GLOBAL_VALUE(x) \
  auto&& CONCAT(_unused, __COUNTER__) __attribute__((unused)) = temporarily_set_global_value(x)

So now when users write:

SCOPED_GLOBAL_VALUE(15);

They get the variable, free of charge, with the expressiveness you wanted.

But of course, there's a caveat. Since we generate the variable name using the pre-processor, we can't use this macro in an inline function. If we do, we'll violate the one definition rule. So that's a thing to consider.

Personally, I wouldn't stress over this. It's a common idiom to require named RAII object (think lock_guard), so just being presented a properly named function would be straight forward for any savvy C++ programmer.

StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
  • Huh? Why not in inline functions? Rules for scope are right the same as for non-inline functions, aren't they? – Aconcagua Jul 01 '18 at 08:02
  • 2
    @Aconcagua - If you include the header which contains the inline function in two different translation units, the variable is likely going to have a different name after each TU is processed. It will violate the ["same exact sequence of tokens"](https://timsong-cpp.github.io/cppwp/basic.def.odr#12.1) requirement. – StoryTeller - Unslander Monica Jul 01 '18 at 08:04
  • 1
    Ah, I see - `static inline` should solve the issue - shouldn't it? Or possibly `__LINE__` instead of `__COUNTER__`? – Aconcagua Jul 01 '18 at 08:33
  • @Aconcagua - `static inline` probably would. But I don't know if there's a point to `inline` then. `__LINE__` may have the same problem, I don't recall relative to what `__LINE__` is calculated, but I think it's the current TU after all the inclusions have been done. I could be wrong. – StoryTeller - Unslander Monica Jul 01 '18 at 08:35
  • test.c: `1: #include 2: void f() { log("something"); }` If now `__LINE__` was resolved *after* inclusions, how could logging systems use it meaningfully (printing `line: 101` instead of `line: 1`)? – Aconcagua Jul 01 '18 at 08:45
  • @Aconcagua - Preprocessors add `#line` directives to keep the original line numbers. And it's all fine and dandy when you use it directly in source. I don't know if it's trustworthy when used in an included header. – StoryTeller - Unslander Monica Jul 01 '18 at 08:49
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/174109/discussion-between-aconcagua-and-storyteller). – Aconcagua Jul 01 '18 at 08:52
1

Sorry about my previous answer folks, what was I thinking? I should have read the question properly.

So, of course, foo() has to return your ResetGuard object in order to extend its lifetime, and this is a good thing, not a bad thing.

Firstly, it's hardly a burden on the caller. After all, all he / she has to do is:

auto rg = foo ();

As a potential caller of foo() I would have absolutely no problem with that, and @melpomene's excellent suggestion in the comments above ([[nodiscard]]) can be used to ensure that callers don't forget to do this.

And why is forcing the caller to do this a good thing (apart from the fact that you have no choice in the matter anyway)? Well, it gives the caller the opportunity to manage the lifetime of the scopeguard and that might be useful (will provide live demo soon).

As for the other answers here, I would most definitely not hide all this in a macro because that hides an important piece of information from potential callers of foo(). Instead, I would use [[nodiscard]] to remind them of their responsibilities and leave it at that.

[Edit]

I have now spent a little time over at Wandbox polishing up the code to add the full set of recommended constructors / assignment operators and to demonstrate the use of [[nodiscard]], which for me is the find of the day.

First, the modified class, done in the way (I believe) those who know are recommending. I can particularly see the importance of defining a proper move constructor (just think of the subtle bugs you might run into if you don't). Pinched some stuff (= delete) from JVApen, looks wise to me, TU JV.

#include <iostream>
#include <assert.h>

#define INCLUDE_COPY_MOVE_SWAP_STUFF

template <class T> class [[nodiscard]] ResetGuard
{
public:
    ResetGuard (T& obj_to_reset, const T& new_value) : old_value (obj_to_reset), obj_to_reset (obj_to_reset)
    {
        obj_to_reset = new_value;
    }

#ifdef INCLUDE_COPY_MOVE_SWAP_STUFF
   ResetGuard (const ResetGuard& copy_from) = delete;
   ResetGuard &operator= (const ResetGuard& copy_assign_from) = delete;
   ResetGuard &operator= (ResetGuard&& move_assign_from) = delete;  

    ResetGuard (ResetGuard&& move_from) : old_value (move_from.old_value), obj_to_reset (move_from.obj_to_reset)
    {
        assert (!move_from.defunct);
        move_from.defunct = true;
    }
#endif

    ~ResetGuard()
    {
        if (!defunct)
            obj_to_reset = old_value;
    }

private:
    T old_value;
    T& obj_to_reset;
    bool defunct = false;
};

Comment out #define INCLUDE_COPY_MOVE_SWAP_STUFF to see the compiler warning you get if you don't do all the things you're supposed to.

Test program:

int GLOBAL_VALUE = 0;

ResetGuard<int> temporarily_set_global_value (int new_val)
{
    return { GLOBAL_VALUE, new_val }; // updates GLOBAL_VALUE
}

void bad_foo()
{
    temporarily_set_global_value (15);
    std::cout << "GLOBAL_VALUE in bad_foo () is " << GLOBAL_VALUE << std::endl;
}

void good_foo()
{
    auto rg = temporarily_set_global_value (15);
    std::cout << "GLOBAL_VALUE in good_foo () is " << GLOBAL_VALUE << std::endl;
}

auto better_foo()
{
    auto rg = temporarily_set_global_value (15);
    std::cout << "GLOBAL_VALUE in better_foo () is " << GLOBAL_VALUE << std::endl;
    return rg;
}

int main ()
{
    bad_foo ();
    good_foo ();
    std::cout << "GLOBAL_VALUE after good_foo () returns is " << GLOBAL_VALUE << std::endl;

    {
        auto rg = better_foo ();
        std::cout << "GLOBAL_VALUE after better_foo () returns is " << GLOBAL_VALUE << std::endl;

        {
            auto rg_moved = std::move (rg);
            std::cout << "GLOBAL_VALUE after ResetGuard moved is " << GLOBAL_VALUE << std::endl;
        }            

        std::cout << "GLOBAL_VALUE after ResetGuard moved to goes out of scope is " << GLOBAL_VALUE << std::endl;
        GLOBAL_VALUE = 42;
    }

    std::cout << "GLOBAL_VALUE after ResetGuard moved from goes out of scope is " << GLOBAL_VALUE << std::endl;
}

Compiler output:

prog.cc: In function 'void bad_foo()':
prog.cc:47:38: warning: ignoring returned value of type 'ResetGuard<int>', declared with attribute nodiscard [-Wunused-result]
     temporarily_set_global_value (15);
                                      ^
prog.cc:40:17: note: in call to 'ResetGuard<int> temporarily_set_global_value(int)', declared here
 ResetGuard<int> temporarily_set_global_value (int new_val)
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
prog.cc:6:40: note: 'ResetGuard<int>' declared here
 template <class T> class [[nodiscard]] ResetGuard
                                        ^~~~~~~~~~

Program output:

GLOBAL_VALUE in bad_foo () is 0
GLOBAL_VALUE in good_foo () is 15
GLOBAL_VALUE after good_foo () returns is 0
GLOBAL_VALUE in better_foo () is 15
GLOBAL_VALUE after better_foo () returns is 15
GLOBAL_VALUE after ResetGuard moved is 15
GLOBAL_VALUE after ResetGuard moved to goes out of scope is 0
GLOBAL_VALUE after ResetGuard moved from goes out of scope is 42 

So there you have it. If you do all the things you're supposed to do (and I hope I have!) then everything works just fine, and it's all nice and efficient thanks to RVO and guaranteed copy elision so there's no need to worry about that either.

Live demo.

Paul Sanders
  • 24,133
  • 4
  • 26
  • 48