facebook / facebook/folly

a test failure in ScopeGuardTest.cpp due to missing "const" specifier on VS2022

Open
#2,158 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
30.5k
Forks
5.9k
PR merge metrics
No merged PRs in 30d

Description

Hi, I found an unexpected failure test case in ScopeGuardTest.cpp on VS2022 today. I'm really confused.

At first, the [TESTThrowingCleanupAction](https://github.com/facebook/folly/blob/main/folly/test/ScopeGuardTest.cpp#L406-L425) test case failed on VS2022 all the time. Then I checked the copy-constructor and found the ScopeGuard use `std::ref` to wrap a throwable functor.

Interestingly, I noticed that msvc adds "const" specifier to their `std::ref` implementations as follows:
```c++
# in msvc's line 2057-2063
public:
template
_CONSTEXPR20 auto operator()(_Types&&... _Args) const // NOTE: const here!!!
noexcept(noexcept(_STD invoke(*_Ptr, static_cast<_Types&&>(_Args)...)))
-> decltype(_STD invoke(*_Ptr, static_cast<_Types&&>(_Args)...)) {
return _STD invoke(*_Ptr, static_cast<_Types&&>(_Args)...);
}
```
I guess `cosnt`/`non-const ` specifier might make things differet. So I added `const` as:
```c++
TEST(ScopeGuard, TESTThrowingCleanupAction) {
struct ThrowingCleanupAction {
// clang-format off
explicit ThrowingCleanupAction(int& scopeExitExecuted)
: scopeExitExecuted_(scopeExitExecuted) {}
[[noreturn]] ThrowingCleanupAction(const ThrowingCleanupAction& other)
: scopeExitExecuted_(other.scopeExitExecuted_) {
throw std::runtime_error("whoa");
}
// clang-format on
void operator()() const { ++scopeExitExecuted_; } // NOTE: add const here

private:
int& scopeExitExecuted_;
};
int scopeExitExecuted = 0;
ThrowingCleanupAction onExit(scopeExitExecuted);
EXPECT_THROW((void)makeGuard(onExit), std::runtime_error);
EXPECT_EQ(scopeExitExecuted, 1);
}
```
And then everything goes just fine.

But I'm confused:
1. Why non-const version failed?
2. The member var `scopeExitExecuted_` could be changed by operator(), why const specifier can be added here? In the past, I thought const before function body means no changes will be made to the members, but it seems incorrect. What does this `const` actually do?
3. Which version should choose if I need to write sth like this? const or non-const version? Does the chosen version has the portability on other platforms?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.