Avoid accidentally creating a temporary

Discussion in 'C++' started by Phil Endecott, May 15, 2011.

  1. Dear Experts,

    Say I have this:

    struct scoped_foo {
    scoped_foo(int n) { enable_foo(n); }
    ~scoped_foo() { disable_foo(); }
    };

    and I use it like this:

    {
    scoped_foo(3);
    blah(); // foo is enabled here
    }
    // foo is disabled here

    ....except that doesn't work, because I should have written

    {
    scoped_foo ANY_NAME_HERE(3);
    ....

    I've now made this mistake a few times, and it's annoying.

    So, my question: is there anything that I can do in my declaration of
    scoped_foo so that trying to use it in this wrong way, i.e. to create
    a temporary rather than an object with scope up to the next }, will
    give an error, or at least a warning (with g++)?

    Thanks for any suggestions!

    Phil.
     
    Phil Endecott, May 15, 2011
    #1
    1. Advertising

  2. On May 15, 12:15 pm, Phil Endecott <> wrote:
    > Dear Experts,
    >
    > Say I have this:
    >
    > struct scoped_foo {
    >   scoped_foo(int n) { enable_foo(n); }
    >   ~scoped_foo() { disable_foo(); }
    >
    > };
    >
    > and I use it like this:
    >
    > {
    >   scoped_foo(3);
    >   blah();  // foo is enabled here}
    >
    > // foo is disabled here
    >
    > ...except that doesn't work, because I should have written
    >
    > {
    >   scoped_foo ANY_NAME_HERE(3);
    >   ....
    >
    > I've now made this mistake a few times, and it's annoying.
    >
    > So, my question: is there anything that I can do in my declaration of
    > scoped_foo so that trying to use it in this wrong way, i.e. to create
    > a temporary rather than an object with scope up to the next }, will
    > give an error, or at least a warning (with g++)?


    This would be a great use for lvalue-ref qualifiers on destructors:

    ~scoped_foo()& { disable_foo(); }

    Unfortunately this use of reference qualifiers is disallowed. :-(

    Perhaps you could propose it.

    Disclaimer: This is off the cuff. I haven't fully investigated such
    proposed use.

    Howard
     
    Howard Hinnant, May 15, 2011
    #2
    1. Advertising

  3. Howard Hinnant wrote:

    > On May 15, 12:15 pm, Phil Endecott <> wrote:
    >> Dear Experts,
    >>
    >> Say I have this:
    >>
    >> struct scoped_foo {
    >> scoped_foo(int n) { enable_foo(n); }
    >> ~scoped_foo() { disable_foo(); }
    >>
    >> };
    >>
    >> and I use it like this:
    >>
    >> {
    >> scoped_foo(3);
    >> blah(); // foo is enabled here}
    >>
    >> // foo is disabled here
    >>
    >> ...except that doesn't work, because I should have written
    >>
    >> {
    >> scoped_foo ANY_NAME_HERE(3);
    >> ....
    >>
    >> I've now made this mistake a few times, and it's annoying.
    >>
    >> So, my question: is there anything that I can do in my declaration of
    >> scoped_foo so that trying to use it in this wrong way, i.e. to create
    >> a temporary rather than an object with scope up to the next }, will
    >> give an error, or at least a warning (with g++)?

    >
    > This would be a great use for lvalue-ref qualifiers on destructors:
    >
    > ~scoped_foo()& { disable_foo(); }
    >
    > Unfortunately this use of reference qualifiers is disallowed. :-(
    >


    We discussed it in #llvm before and Doug found it's an interesting idea.
    Also see http://stackoverflow.com/questions/4850674/is-it-possible-to-
    restrict-class-instances-to-be-used-only-as-temporaries for another guy
    desperately looking for something like this, but in the opposite direction
    ("&&" instead of "&") :)
     
    Johannes Schaub, May 15, 2011
    #3
  4. Phil Endecott

    Öö Tiib Guest

    On May 15, 7:15 pm, Phil Endecott <> wrote:
    > Dear Experts,
    >
    > Say I have this:
    >
    > struct scoped_foo {
    >   scoped_foo(int n) { enable_foo(n); }
    >   ~scoped_foo() { disable_foo(); }
    >
    > };
    >
    > and I use it like this:
    >
    > {
    >   scoped_foo(3);
    >   blah();  // foo is enabled here}
    >
    > // foo is disabled here
    >
    > ...except that doesn't work, because I should have written
    >
    > {
    >   scoped_foo ANY_NAME_HERE(3);
    >   ....
    >
    > I've now made this mistake a few times, and it's annoying.
    >
    > So, my question: is there anything that I can do in my declaration of
    > scoped_foo so that trying to use it in this wrong way, i.e. to create
    > a temporary rather than an object with scope up to the next }, will
    > give an error, or at least a warning (with g++)?
    >
    > Thanks for any suggestions!


    Stop using magic numbers.

    #include "your_scoped_foo.h"

    const int kaka = 3;

    int main()
    {
      scoped_foo(kaka); // no default constructor for scoped_foo
    }

    ;)
     
    Öö Tiib, May 15, 2011
    #4
  5. * Phil Endecott, on 15.05.2011 18:15:
    > Dear Experts,
    >
    > Say I have this:
    >
    > struct scoped_foo {
    > scoped_foo(int n) { enable_foo(n); }
    > ~scoped_foo() { disable_foo(); }
    > };
    >
    > and I use it like this:
    >
    > {
    > scoped_foo(3);
    > blah(); // foo is enabled here
    > }
    > // foo is disabled here
    >
    > ...except that doesn't work, because I should have written
    >
    > {
    > scoped_foo ANY_NAME_HERE(3);
    > ....
    >
    > I've now made this mistake a few times, and it's annoying.
    >
    > So, my question: is there anything that I can do in my declaration of
    > scoped_foo so that trying to use it in this wrong way, i.e. to create
    > a temporary rather than an object with scope up to the next }, will
    > give an error, or at least a warning (with g++)?
    >
    > Thanks for any suggestions!


    How about


    <code>
    #include <stdio.h>
    #include <assert.h>

    void enable_foo( int ) { printf( "e\n" ); }
    void disable_foo() { printf( "d\n" ); }

    namespace detail
    {
    struct scoped_foo_impl
    {
    scoped_foo_impl( scoped_foo_impl const& )
    {
    assert( "Ouch, copy constructing! Probably no RVO..." );
    }

    explicit scoped_foo_impl( int n )
    {
    enable_foo( n );
    }

    ~scoped_foo_impl() { disable_foo(); }
    };

    scoped_foo_impl make_foo( int n ) { return scoped_foo_impl( n ); }
    }

    typedef detail::scoped_foo_impl const& scoped_foo;
    using detail::make_foo;

    int main()
    {
    scoped_foo g = make_foo( 3 );
    //scoped_foo make_foo( 3 ); // Nix!
    //scoped_foo( 3 ); // Nyet!

    printf( "*\n" );
    }
    </code>


    Cheers & hth.,


    - Alf "why didn't they ask me"

    --
    blog at <url: http://alfps.wordpress.com>
     
    Alf P. Steinbach /Usenet, May 15, 2011
    #5
  6. * Alf P. Steinbach /Usenet, on 15.05.2011 19:50:
    > * Phil Endecott, on 15.05.2011 18:15:
    >> Dear Experts,
    >>
    >> Say I have this:
    >>
    >> struct scoped_foo {
    >> scoped_foo(int n) { enable_foo(n); }
    >> ~scoped_foo() { disable_foo(); }
    >> };
    >>
    >> and I use it like this:
    >>
    >> {
    >> scoped_foo(3);
    >> blah(); // foo is enabled here
    >> }
    >> // foo is disabled here
    >>
    >> ...except that doesn't work, because I should have written
    >>
    >> {
    >> scoped_foo ANY_NAME_HERE(3);
    >> ....
    >>
    >> I've now made this mistake a few times, and it's annoying.
    >>
    >> So, my question: is there anything that I can do in my declaration of
    >> scoped_foo so that trying to use it in this wrong way, i.e. to create
    >> a temporary rather than an object with scope up to the next }, will
    >> give an error, or at least a warning (with g++)?
    >>
    >> Thanks for any suggestions!

    >
    > How about
    >
    >
    > <code>
    > #include <stdio.h>
    > #include <assert.h>
    >
    > void enable_foo( int ) { printf( "e\n" ); }
    > void disable_foo() { printf( "d\n" ); }
    >
    > namespace detail
    > {
    > struct scoped_foo_impl
    > {
    > scoped_foo_impl( scoped_foo_impl const& )
    > {
    > assert( "Ouch, copy constructing! Probably no RVO..." );
    > }
    >
    > explicit scoped_foo_impl( int n )
    > {
    > enable_foo( n );
    > }
    >
    > ~scoped_foo_impl() { disable_foo(); }
    > };
    >
    > scoped_foo_impl make_foo( int n ) { return scoped_foo_impl( n ); }
    > }
    >
    > typedef detail::scoped_foo_impl const& scoped_foo;
    > using detail::make_foo;
    >
    > int main()
    > {
    > scoped_foo g = make_foo( 3 );
    > //scoped_foo make_foo( 3 ); // Nix!
    > //scoped_foo( 3 ); // Nyet!
    >
    > printf( "*\n" );
    > }
    > </code>
    >
    >
    > Cheers & hth.,
    >
    >
    > - Alf "why didn't they ask me"
    >


    oh, forgot an "&& false" or other such device in the assert. just add


    --
    blog at <url: http://alfps.wordpress.com>
     
    Alf P. Steinbach /Usenet, May 15, 2011
    #6
  7. On May 15, 5:15 pm, Phil Endecott <> wrote:
    > struct scoped_foo {
    > scoped_foo(int n) { enable_foo(n); }
    > ~scoped_foo() { disable_foo(); }
    > };


    > is there anything that I can do in my declaration of
    > scoped_foo so that trying to use it in this wrong way, i.e. to create
    > a temporary rather than an object with scope up to the next }, will
    > give an error, or at least a warning (with g++)?


    I guess I want something like this:

    scoped_foo(int n) __attribute__((warn_unused_result))
    { enable_foo(n); }

    which doesn't work, but perhaps there's something similar that I can
    do.

    BTW, one case where this could really hurt is scoped locks:

    {
    Lock(mutex); // Nope. But I could stare at it for hours and not
    spot the error.
    modify(shared_state); // Ooops. Hard to debug that.
    }


    Phil.
     
    Phil Endecott, May 15, 2011
    #7
  8. * Phil Endecott, on 15.05.2011 20:33:
    > On May 15, 5:15 pm, Phil Endecott<> wrote:
    >> struct scoped_foo {
    >> scoped_foo(int n) { enable_foo(n); }
    >> ~scoped_foo() { disable_foo(); }
    >> };

    >
    >> is there anything that I can do in my declaration of
    >> scoped_foo so that trying to use it in this wrong way, i.e. to create
    >> a temporary rather than an object with scope up to the next }, will
    >> give an error, or at least a warning (with g++)?

    >
    > I guess I want something like this:
    >
    > scoped_foo(int n) __attribute__((warn_unused_result))
    > { enable_foo(n); }
    >
    > which doesn't work, but perhaps there's something similar that I can
    > do.
    >
    > BTW, one case where this could really hurt is scoped locks:
    >
    > {
    > Lock(mutex); // Nope. But I could stare at it for hours and not
    > spot the error.
    > modify(shared_state); // Ooops. Hard to debug that.
    > }


    Why are you replying to your own post?

    Anyway, see my reply.

    And also, you might want to look up the ScopeGuard article by Marginean and
    Alexandrescu in DDJ.


    Cheers & hth.,

    - Alf "baffled"

    --
    blog at <url: http://alfps.wordpress.com>
     
    Alf P. Steinbach /Usenet, May 15, 2011
    #8
  9. Alf P. Steinbach /Usenet wrote:

    > * Phil Endecott, on 15.05.2011 18:15:
    >> Dear Experts,
    >>
    >> Say I have this:
    >>
    >> struct scoped_foo {
    >> scoped_foo(int n) { enable_foo(n); }
    >> ~scoped_foo() { disable_foo(); }
    >> };
    >>
    >> and I use it like this:
    >>
    >> {
    >> scoped_foo(3);
    >> blah(); // foo is enabled here
    >> }
    >> // foo is disabled here
    >>
    >> ...except that doesn't work, because I should have written
    >>
    >> {
    >> scoped_foo ANY_NAME_HERE(3);
    >> ....
    >>
    >> I've now made this mistake a few times, and it's annoying.
    >>
    >> So, my question: is there anything that I can do in my declaration of
    >> scoped_foo so that trying to use it in this wrong way, i.e. to create
    >> a temporary rather than an object with scope up to the next }, will
    >> give an error, or at least a warning (with g++)?
    >>
    >> Thanks for any suggestions!

    >
    > How about
    >
    >
    > <code>
    > #include <stdio.h>
    > #include <assert.h>
    >
    > void enable_foo( int ) { printf( "e\n" ); }
    > void disable_foo() { printf( "d\n" ); }
    >
    > namespace detail
    > {
    > struct scoped_foo_impl
    > {
    > scoped_foo_impl( scoped_foo_impl const& )
    > {
    > assert( "Ouch, copy constructing! Probably no RVO..." );
    > }
    >
    > explicit scoped_foo_impl( int n )
    > {
    > enable_foo( n );
    > }
    >
    > ~scoped_foo_impl() { disable_foo(); }
    > };
    >
    > scoped_foo_impl make_foo( int n ) { return scoped_foo_impl( n );
    > }
    > }
    >
    > typedef detail::scoped_foo_impl const& scoped_foo;
    > using detail::make_foo;
    >
    > int main()
    > {
    > scoped_foo g = make_foo( 3 );
    > //scoped_foo make_foo( 3 ); // Nix!
    > //scoped_foo( 3 ); // Nyet!
    >
    > printf( "*\n" );
    > }
    > </code>
    >
    >


    This is a neat trick, I will have to remember it. Sadly, it doesn't work
    with list initialization

    scoped_foo make_foo{ 3 }; // valid
    scoped_foo { 3 }; // valid :(
     
    Johannes Schaub, May 15, 2011
    #9
  10. * Johannes Schaub, on 15.05.2011 20:49:
    > Alf P. Steinbach /Usenet wrote:
    >
    >> * Phil Endecott, on 15.05.2011 18:15:
    >>> Dear Experts,
    >>>
    >>> Say I have this:
    >>>
    >>> struct scoped_foo {
    >>> scoped_foo(int n) { enable_foo(n); }
    >>> ~scoped_foo() { disable_foo(); }
    >>> };
    >>>
    >>> and I use it like this:
    >>>
    >>> {
    >>> scoped_foo(3);
    >>> blah(); // foo is enabled here
    >>> }
    >>> // foo is disabled here
    >>>
    >>> ...except that doesn't work, because I should have written
    >>>
    >>> {
    >>> scoped_foo ANY_NAME_HERE(3);
    >>> ....
    >>>
    >>> I've now made this mistake a few times, and it's annoying.
    >>>
    >>> So, my question: is there anything that I can do in my declaration of
    >>> scoped_foo so that trying to use it in this wrong way, i.e. to create
    >>> a temporary rather than an object with scope up to the next }, will
    >>> give an error, or at least a warning (with g++)?
    >>>
    >>> Thanks for any suggestions!

    >>
    >> How about
    >>
    >>
    >> <code>
    >> #include<stdio.h>
    >> #include<assert.h>
    >>
    >> void enable_foo( int ) { printf( "e\n" ); }
    >> void disable_foo() { printf( "d\n" ); }
    >>
    >> namespace detail
    >> {
    >> struct scoped_foo_impl
    >> {
    >> scoped_foo_impl( scoped_foo_impl const& )
    >> {
    >> assert( "Ouch, copy constructing! Probably no RVO..." );
    >> }
    >>
    >> explicit scoped_foo_impl( int n )
    >> {
    >> enable_foo( n );
    >> }
    >>
    >> ~scoped_foo_impl() { disable_foo(); }
    >> };
    >>
    >> scoped_foo_impl make_foo( int n ) { return scoped_foo_impl( n );
    >> }
    >> }
    >>
    >> typedef detail::scoped_foo_impl const& scoped_foo;
    >> using detail::make_foo;
    >>
    >> int main()
    >> {
    >> scoped_foo g = make_foo( 3 );
    >> //scoped_foo make_foo( 3 ); // Nix!
    >> //scoped_foo( 3 ); // Nyet!
    >>
    >> printf( "*\n" );
    >> }
    >> </code>
    >>
    >>

    >
    > This is a neat trick, I will have to remember it. Sadly, it doesn't work
    > with list initialization
    >
    > scoped_foo make_foo{ 3 }; // valid
    > scoped_foo { 3 }; // valid :(
    >
    >


    C++0x is off-topic... ;-)

    Cheers,

    - Alf

    --
    blog at <url: http://alfps.wordpress.com>
     
    Alf P. Steinbach /Usenet, May 15, 2011
    #10
  11. On May 15, 7:39 pm, "Alf P. Steinbach /Usenet" <alf.p.steinbach
    > wrote:
    > Why are you replying to your own post?


    Because I'm adding more information that's not related to any of the
    replies. Which post would you prefer me to reply to?

    > Anyway, see my reply.


    Your suggestion requires that I change all the places where I use the
    class; that's not ideal.

    > And also, you might want to look up the ScopeGuard article by Marginean and
    > Alexandrescu in DDJ.


    You mean http://drdobbs.com/cpp/184403758 ? I guess the relevant bit
    is that they define a macro that hides some of this. Yes, I guess I
    could do that:

    #define SCOPED_FOO(P) scoped_foo F_#LINE(P)

    ....or similar, I forget the LINE details. Thanks for the suggestion.


    Phil.
     
    Phil Endecott, May 15, 2011
    #11
  12. Phil Endecott

    Marc Guest

    Johannes Schaub wrote:

    > Alf P. Steinbach /Usenet wrote:
    >
    >> <code>
    >> #include <stdio.h>
    >> #include <assert.h>
    >>
    >> void enable_foo( int ) { printf( "e\n" ); }
    >> void disable_foo() { printf( "d\n" ); }
    >>
    >> namespace detail
    >> {
    >> struct scoped_foo_impl
    >> {
    >> scoped_foo_impl( scoped_foo_impl const& )
    >> {
    >> assert( "Ouch, copy constructing! Probably no RVO..." );
    >> }
    >>
    >> explicit scoped_foo_impl( int n )
    >> {
    >> enable_foo( n );
    >> }
    >>
    >> ~scoped_foo_impl() { disable_foo(); }
    >> };
    >>
    >> scoped_foo_impl make_foo( int n ) { return scoped_foo_impl( n );
    >> }
    >> }
    >>
    >> typedef detail::scoped_foo_impl const& scoped_foo;
    >> using detail::make_foo;
    >>
    >> int main()
    >> {
    >> scoped_foo g = make_foo( 3 );
    >> //scoped_foo make_foo( 3 ); // Nix!
    >> //scoped_foo( 3 ); // Nyet!
    >>
    >> printf( "*\n" );
    >> }
    >> </code>

    >
    > This is a neat trick, I will have to remember it. Sadly, it doesn't work
    > with list initialization
    >
    > scoped_foo make_foo{ 3 }; // valid
    > scoped_foo { 3 }; // valid :(


    If you're introducing a factory:

    class A; A make_A(int n);

    class A {
    // implicit private:
    A(int){}
    friend A make_A(int n){return A(n);}
    };
    int main(){
    A a=make_A(3);
    }

    (you can still couple that with the various tricks above)

    Easiest remains the macro:
    #define takefoo(X) scoped_foo foo##__LINE__ (X);
     
    Marc, May 15, 2011
    #12
  13. * Phil Endecott, on 15.05.2011 21:28:
    > On May 15, 7:39 pm, "Alf P. Steinbach /Usenet"<alf.p.steinbach
    > > wrote:
    >> Why are you replying to your own post?

    >
    > Because I'm adding more information that's not related to any of the
    > replies. Which post would you prefer me to reply to?


    I'd prefer that you start new threads for new problems.


    >> Anyway, see my reply.

    >
    > Your suggestion requires that I change all the places where I use the
    > class; that's not ideal.


    If you want to check whether existing usages are wrong (just temporaries), then
    that's a different problem.

    AFAIK it has no solution within the C++ language.

    Instead you'd have to parse your C++ code, manually or by creating some tool to
    do it. I would suggest just grepping the lines with the class name, then
    checking those lines. It would not pick up typedefs or macros etc., but as a
    practical matter it would probably suffice for what you now say the problem is.


    >> And also, you might want to look up the ScopeGuard article by Marginean and
    >> Alexandrescu in DDJ.

    >
    > You mean http://drdobbs.com/cpp/184403758 ? I guess the relevant bit
    > is that they define a macro that hides some of this. Yes, I guess I
    > could do that:
    >
    > #define SCOPED_FOO(P) scoped_foo F_#LINE(P)
    >
    > ...or similar, I forget the LINE details. Thanks for the suggestion.


    You're welcome.

    However, note that also scopeguard macros requires changing the places where
    you're using the class, so wrt. your criteria it's "not ideal".

    Also, note that MSVC with edit-and-continue option Z-something is very
    non-standard wrt. the __LINE__ macro, so Marginean's code needs to be
    special-cased for MSVC (using e.g. MSVC's language extension __COUNTER__).


    Cheers & hth.,

    - Alf "not surprised"

    --
    blog at <url: http://alfps.wordpress.com>
     
    Alf P. Steinbach /Usenet, May 15, 2011
    #13
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. sudha
    Replies:
    1
    Views:
    319
  2. Alexander Malkis
    Replies:
    8
    Views:
    551
    Alexander Malkis
    Apr 14, 2004
  3. Roger23
    Replies:
    2
    Views:
    1,039
    Roger23
    Oct 12, 2006
  4. Replies:
    7
    Views:
    3,335
    James Kanze
    Feb 12, 2008
  5. Evgenii Sputnik
    Replies:
    3
    Views:
    188
    Arne Vajhøj
    Feb 1, 2013
Loading...

Share This Page