Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1760453004:
Add operator* and operator safe-bool to sk_sp. (Closed)
|
Created:
4 years, 9 months ago by bungeman-skia Modified:
4 years, 9 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionAdd operator* and operator safe-bool to sk_sp.
This greatly reduces the need to use '.get()' in conditionals.
Committed: https://skia.googlesource.com/skia/+/beab9e7ba06ea8bd01fcbfd44f2ca2ed69c8a1d9
Patch Set 1 #Patch Set 2 : Correct signature, write test. #
Total comments: 5
Patch Set 3 : Test all the things. #
Messages
Total messages: 35 (18 generated)
Description was changed from ========== Add operator* and operator safe-bool to sk_sp. ========== to ========== Add operator* and operator safe-bool to sk_sp. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Description was changed from ========== Add operator* and operator safe-bool to sk_sp. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add operator* and operator safe-bool to sk_sp. ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1760453004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1760453004/1
bungeman@google.com changed reviewers: + halcanary@google.com, mtklein@google.com, reed@google.com
This means we don't have to use '.get()' everywhere.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Mac-Clang-Arm7-Debug-iOS-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-Arm7...)
Description was changed from ========== Add operator* and operator safe-bool to sk_sp. ========== to ========== Add operator* and operator safe-bool to sk_sp. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1760453004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1760453004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Mac-Clang-Arm7-Debug-iOS-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-Arm7...)
Description was changed from ========== Add operator* and operator safe-bool to sk_sp. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add operator* and operator safe-bool to sk_sp. This greatly reduces the need to use '.get()' in conditionals. ==========
lgtm
https://codereview.chromium.org/1760453004/diff/20001/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): https://codereview.chromium.org/1760453004/diff/20001/include/core/SkRefCnt.h... include/core/SkRefCnt.h:241: using unspecified_bool_type = T* sk_sp::*; // safe bool idiom. obsolete with explicit operator bool https://codereview.chromium.org/1760453004/diff/20001/include/core/SkRefCnt.h... include/core/SkRefCnt.h:329: operator unspecified_bool_type() const { return this->get() ? &sk_sp::fPtr : nullptr; } ... ? &fPtr : nullptr ... ? https://codereview.chromium.org/1760453004/diff/20001/tests/RefCntTest.cpp File tests/RefCntTest.cpp (right): https://codereview.chromium.org/1760453004/diff/20001/tests/RefCntTest.cpp#ne... tests/RefCntTest.cpp:195: delete (*paint.get()).method(); // tests operator*
is it at all useful to explicit exercise if (!obj.get()) ... ?
Description was changed from ========== Add operator* and operator safe-bool to sk_sp. This greatly reduces the need to use '.get()' in conditionals. ========== to ========== Add operator* and operator safe-bool to sk_sp. This greatly reduces the need to use '.get()' in conditionals. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
> is it at all useful to explicit exercise > > if (!obj.get()) ... ? Done. https://codereview.chromium.org/1760453004/diff/20001/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): https://codereview.chromium.org/1760453004/diff/20001/include/core/SkRefCnt.h... include/core/SkRefCnt.h:241: using unspecified_bool_type = T* sk_sp::*; On 2016/03/02 20:05:10, mtklein wrote: > // safe bool idiom. obsolete with explicit operator bool Done. https://codereview.chromium.org/1760453004/diff/20001/include/core/SkRefCnt.h... include/core/SkRefCnt.h:329: operator unspecified_bool_type() const { return this->get() ? &sk_sp::fPtr : nullptr; } On 2016/03/02 20:05:10, mtklein wrote: > ... ? &fPtr : nullptr ... > ? So it turns out the type of '&fPtr' here is 'T*const*' but '&sk_sp::fPtr' is 'T* sk_sp<T>::*' (which is what we actually want). The difference is that the first is the address of the fPtr object in memory, the other is a field pointer which may be used to find the fPtr field of sk_sp<T> objects (note that this means that all instances of sk_sp<T> for a given T will share the same value with the same type).
Description was changed from ========== Add operator* and operator safe-bool to sk_sp. This greatly reduces the need to use '.get()' in conditionals. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add operator* and operator safe-bool to sk_sp. This greatly reduces the need to use '.get()' in conditionals. ==========
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1760453004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1760453004/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Mac-Clang-Arm7-Debug-iOS-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-Arm7...)
lgtm "g" in wary quotes
The CQ bit was checked by bungeman@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from halcanary@google.com Link to the patchset: https://codereview.chromium.org/1760453004/#ps40001 (title: "Test all the things.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1760453004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1760453004/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED, http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)
api lgtm
The CQ bit was checked by bungeman@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1760453004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1760453004/40001
Message was sent while issue was closed.
Description was changed from ========== Add operator* and operator safe-bool to sk_sp. This greatly reduces the need to use '.get()' in conditionals. ========== to ========== Add operator* and operator safe-bool to sk_sp. This greatly reduces the need to use '.get()' in conditionals. Committed: https://skia.googlesource.com/skia/+/beab9e7ba06ea8bd01fcbfd44f2ca2ed69c8a1d9 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/beab9e7ba06ea8bd01fcbfd44f2ca2ed69c8a1d9 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
