Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
497723004:
Remove implicit conversions from scoped_refptr to T* in components/search_provider_logos/ (Closed)
DescriptionRemove implicit conversions from scoped_refptr to T* in components/search_provider_logos/
This patch was generated by running the rewrite_scoped_refptr clang tool
on a Linux build.
BUG=110610
Committed: https://crrev.com/1d192de14e8d0a22a4f3bcf30497f2814484c7b1
Cr-Commit-Position: refs/heads/master@{#292232}
Patch Set 1 #
Messages
Total messages: 31 (0 generated)
dcheng@chromium.org changed reviewers: + justincohen@chromium.org
Unfortunately, we can't make scoped_refptr testable until the conversion
operator has been removed. While it would be possible to disable the conversion
operator and make it testable in the same patch, the automated tool tries to
skip rewriting code where manual cleanups would be useful. This includes things
like:
// What if ReturnsScopedRefPtr() creates a new object internally?
if (ReturnsScopedRefPtr())
Foo* Bar() {
const scoped_refptr<Foo>& f = ReturnsScopedRefPtr();
return f; // Does the return type need to be rewritten? Or is .get() safe?
}
Doing this in one patch would make catching some of these instances (in
particular, a boolean test on a temporary scoped_refptr) more difficult. As a
result, the work is being split into several steps:
Step 1: Rewrite all implicit conversions to be explicit (or rewrite the receiver
type to scoped_refptr<T>, if the tool believes the conversion to be unsafe).
Step 2: Once a platform compiles cleanly without the conversion operator, #ifdef
the conversion operator out.
Step 3: Make scoped_refptr testable on that platform.
Step 4: Remove unnecessary calls to .get().
lgtm
The CQ bit was checked by dcheng@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dcheng@chromium.org/497723004/1
The CQ bit was unchecked by commit-bot@chromium.org
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an LGTM may have been provided, it was from a non-committer or a provisional committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
dcheng@chromium.org changed reviewers: + newt@chromium.org
+newt for OWNERS stamp.
lgtm
The CQ bit was checked by dcheng@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dcheng@chromium.org/497723004/1
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_dbg_tests_recipe on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg_tests_recipe on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
The CQ bit was checked by dcheng@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dcheng@chromium.org/497723004/1
The CQ bit was unchecked by commit-bot@chromium.org
Failed to commit the patch.
The CQ bit was checked by dcheng@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dcheng@chromium.org/497723004/1
The CQ bit was unchecked by commit-bot@chromium.org
Failed to commit the patch.
The CQ bit was checked by dcheng@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dcheng@chromium.org/497723004/1
The CQ bit was unchecked by commit-bot@chromium.org
Failed to commit the patch.
The CQ bit was checked by dcheng@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dcheng@chromium.org/497723004/1
Message was sent while issue was closed.
Committed patchset #1 (id:1) as c0c41aaa1c0cb056a5cb783edb1eb19020c6ed86
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/1d192de14e8d0a22a4f3bcf30497f2814484c7b1 Cr-Commit-Position: refs/heads/master@{#292232} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
