Use explicit operator bool for smart pointer boolean testing |
|||
Issue description
Right now, explicit conversion operators aren't allowed since VC++2013 doesn't actually enforce the explicit part. But on Windows, maybe we have sufficient coverage from the clang-cl builds to be OK with using this?
Even given the VC++2013 limitations, I think we should still consider transitioning now: the current idiom that smart pointers use to make themselves testable allows them to convert to bools in places where a contextual conversion to bool wouldn't normally apply. In the process of migrating from scoped_ptr to std::unique_ptr, we've already found a few bugs:
Example 1:
void BoundBoolSetFromScopedPtrFreeDeleter(
bool* var,
scoped_ptr<bool, base::FreeDeleter> val) {
*var = val; // should be *val!
}
Example 2:
bool HidReceiveFunction::ValidateParameters() {
parameters_ = hid::Receive::Params::Create(*args_);
// Function is supposed to return true if parameters are OK.
// But this macro returns a non-null scoped_ptr on validation
// failure, which converts to true, which is the opposite of
// what we want!
EXTENSION_FUNCTION_VALIDATE(parameters_);
...
}
(I hear VC++2015 is around the corner, so maybe the VC++2013 compat question won't matter for too much longer)
,
Mar 7 2016
With luck the switch to VS 2015 will happen this week. Famous last words. clang-cl has a few bots building Chromium, but they are all (IIRC) FYI bots, so breakage may not be noticed immediately. For instance: https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin or search for ClangToTWin on https://build.chromium.org/p/chromium.fyi/builders
,
Mar 7 2016
They are FYI builders, but I think at least a few people (at the very least, I'm guessing thakis, hans, and me) compile it on a regular basis and fix compile breakages. But it sounds like we're close enough that it's just a matter of a few more days, so in that case, it doesn't really hurt to wait.
,
Mar 8 2017
This issue has been available for more than 365 days, and should be re-evaluated. Please re-triage this issue. The Hotlist-Recharge-Cold label is applied for tracking purposes, and should not be removed after re-triaging the issue. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 9 2017
I can't find any reference to UnspecifiedBoolType (which IIRC was our old idiom for this) in Chromium (aside from third_party). std::unique_ptr, scoped_refptr and WTF::RefPtr (the common smart pointers that come to mind) use "explicit operator bool" today. So I'm gonna close this as obsolete, unless there's still more to do here. |
|||
►
Sign in to add a comment |
|||
Comment 1 by danakj@chromium.org
, Mar 3 2016