Skip to content

Conversation

@davmason
Copy link
Contributor

Because there was not a named variable some compilers were optimizing out the RAII guard.

@ghost
Copy link

ghost commented Jan 20, 2021

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Because there was not a named variable some compilers were optimizing out the RAII guard.

Author: davmason
Assignees: davmason
Labels:

area-Diagnostics-coreclr

Milestone: -

@davmason davmason linked an issue Jan 20, 2021 that may be closed by this pull request
Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh man...

@davmason davmason linked an issue Jan 20, 2021 that may be closed by this pull request
@davmason davmason merged commit 82bbb01 into dotnet:master Jan 20, 2021
@GSPP
Copy link

GSPP commented Jan 28, 2021

Is this a legal optimization? It seems not.

@davmason
Copy link
Contributor Author

@GSPP With the first version ShutdownGuard(); it was introducing a temporary and after the change ShutdownGuard shutdownGuard; it is introducing a named variable. The standard says in 3.7.3 that:

If a variable with automatic storage duration has initialization or a destructor with side effects, it shall not be destroyed before the end of its block, nor shall it be eliminated as an optimization even if it appears to be unused, except that a class object or its copy/move may be eliminated as specified in 12.8.

The key here is that the temporary does not have automatic storage duration, but the named variable does. So it seems like a perfectly legal optimization to me

@GSPP
Copy link

GSPP commented Jan 31, 2021

Thanks for explaining. This was beyond my knowledge of C++.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test failed: profiler/unittest/releaseondetach/releaseondetach.sh Test failure : profiler/elt/slowpatheltleave/slowpatheltleave.sh

3 participants