a test failure in ScopeGuardTest.cpp due to missing "const" specifier on VS2022
- 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
Assessment
This issue has not been assessed yet.