New issue
Advanced search Search tips

Issue 591741 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Use explicit operator bool for smart pointer boolean testing

Project Member Reported by dcheng@chromium.org, Mar 3 2016

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)
 
What coverage do we have from clang win?
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

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.
Project Member

Comment 4 by sheriffbot@chromium.org, Mar 8 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
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
Status: WontFix (was: Untriaged)
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