Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design of Final_act is dependent on copy elision to avoid multiple 'final acts' #11

Closed
jbcoe opened this issue Sep 19, 2015 · 3 comments
Assignees
Labels

Comments

@jbcoe
Copy link

jbcoe commented Sep 19, 2015

The move constructor does not flag the moved-from object as non-invokable

auto invocation_count = 0;
auto f = finally([&invocation_count]{ ++invocation_count;});
assert(invocation_count == 1);

If the copy-by-rvalue-reference is not elided (which is possible but not necessary) then the invocation count will be 2. The final_act object needs an internal bool to track its state or the C++ standard needs to require copy elision.

See: https://github.com/jbcoe/CppSandbox/blob/master/ScopeGuard/ScopeGuard.cpp

for a similar class.

@neilmacintosh
Copy link
Collaborator

That's a great point Jonathan. This version of Final_act was just meant to be a initial sketch, and I had a suspicion we'd miss some subtleties in it, like the risk from a lack of copy elision that you point out here.

It would be terrific if you wanted to contribute a PR that improved Final_act to be free of this defect...

@jbcoe
Copy link
Author

jbcoe commented Sep 23, 2015

I've added a patch already to fix this and #12

@neilmacintosh
Copy link
Collaborator

This has now been fixed with PR #90.

clrxbl added a commit to clrxbl/GSL that referenced this issue Oct 8, 2018
Test project /GSL
      Start  1: span_tests
 1/15 Test  microsoft#1: span_tests .......................   Passed    0.01 sec
      Start  2: multi_span_tests
 2/15 Test  microsoft#2: multi_span_tests .................   Passed    0.02 sec
      Start  3: strided_span_tests
 3/15 Test  microsoft#3: strided_span_tests ...............   Passed    0.01 sec
      Start  4: string_span_tests
 4/15 Test  microsoft#4: string_span_tests ................   Passed    0.01 sec
      Start  5: at_tests
 5/15 Test  microsoft#5: at_tests .........................   Passed    0.00 sec
      Start  6: bounds_tests
 6/15 Test  microsoft#6: bounds_tests .....................   Passed    0.01 sec
      Start  7: notnull_tests
 7/15 Test  microsoft#7: notnull_tests ....................   Passed    0.01 sec
      Start  8: assertion_tests
 8/15 Test  microsoft#8: assertion_tests ..................   Passed    0.01 sec
      Start  9: utils_tests
 9/15 Test  microsoft#9: utils_tests ......................   Passed    0.01 sec
      Start 10: owner_tests
10/15 Test microsoft#10: owner_tests ......................   Passed    0.00 sec
      Start 11: byte_tests
11/15 Test microsoft#11: byte_tests .......................   Passed    0.01 sec
      Start 12: algorithm_tests
12/15 Test microsoft#12: algorithm_tests ..................   Passed    0.00 sec
      Start 13: sloppy_notnull_tests
13/15 Test microsoft#13: sloppy_notnull_tests .............   Passed    0.00 sec
      Start 14: no_exception_throw_tests
14/15 Test microsoft#14: no_exception_throw_tests .........   Passed    0.00 sec
      Start 15: no_exception_ensure_tests
15/15 Test microsoft#15: no_exception_ensure_tests ........   Passed    0.00 sec

100% tests passed, 0 tests failed out of 15

Total Test time (real) =   0.11 sec

Arch Linux w/ Clang 3.12.3
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants