Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1814153003:
Templatize SkToXXX. (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. |
DescriptionTemplatize SkToXXX.
Makes the checked cast in debug more correct, avoiding new
warnings in vs2015.
BUG=skia:4553
Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320
Committed: https://skia.googlesource.com/skia/+/68c14d9b3244ef9d58727ff11f2742be9236f46e
Patch Set 1 #Patch Set 2 : Fix unused variable. #
Total comments: 2
Patch Set 3 : Workaround for old libstdc++. #Patch Set 4 : Chromium actually uses 4.6.4. #
Messages
Total messages: 41 (23 generated)
Description was changed from ========== Templatize SkToXXX. BUG=skia:4553 ========== to ========== Templatize SkToXXX. BUG=skia:4553 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Templatize SkToXXX. BUG=skia:4553 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Templatize SkToXXX. BUG=skia:4553 ==========
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/1814153003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1814153003/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...) Build-Ubuntu-GCC-Mips-Debug-Android-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Mip...)
Description was changed from ========== Templatize SkToXXX. BUG=skia:4553 ========== to ========== Templatize SkToXXX. BUG=skia:4553 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/1814153003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1814153003/20001
Description was changed from ========== Templatize SkToXXX. BUG=skia:4553 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Templatize SkToXXX. BUG=skia:4553 ==========
Description was changed from ========== Templatize SkToXXX. BUG=skia:4553 ========== to ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 ==========
bungeman@google.com changed reviewers: + mtklein@google.com, reed@google.com
This change is already too large, but in the future we should probably move SkToXXX to private and get the android users off of it. (That being said, SkTypes.h might want to be private too, but that's a much larger thing.)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/1814153003/diff/20001/include/core/SkTypes.h File include/core/SkTypes.h (right): https://codereview.chromium.org/1814153003/diff/20001/include/core/SkTypes.h#... include/core/SkTypes.h:261: SkASSERT(SkTFitsIn<D>(s)); Remind me why this can't be, SkASSERT(std::numeric_limits<D>::min() <= s && s <= std::numeric_limits<D>::max()); ?
https://codereview.chromium.org/1814153003/diff/20001/include/core/SkTypes.h File include/core/SkTypes.h (right): https://codereview.chromium.org/1814153003/diff/20001/include/core/SkTypes.h#... include/core/SkTypes.h:261: SkASSERT(SkTFitsIn<D>(s)); On 2016/03/18 16:37:46, mtklein wrote: > Remind me why this can't be, > > SkASSERT(std::numeric_limits<D>::min() <= s && s <= > std::numeric_limits<D>::max()); > > ? This doesn't work (correctly) with enum types (I added some extra logic to SkTFitsIn to deal with that). This also has issues with mixes of signed and unsigned since if D is unsigned, s will be made unsigned before the comparison.
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/1814153003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1814153003/20001
Message was sent while issue was closed.
Description was changed from ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 ========== to ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1820433002/ by bungeman@google.com. The reason for reverting is: Chrome does not yet have std::underlying_type..
Message was sent while issue was closed.
Description was changed from ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 ========== to ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Message was sent while issue was closed.
Description was changed from ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 ==========
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/1814153003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1814153003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 ========== to ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 ==========
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/1814153003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1814153003/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by bungeman@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com, reed@google.com Link to the patchset: https://codereview.chromium.org/1814153003/#ps60001 (title: "Chromium actually uses 4.6.4.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1814153003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1814153003/60001
Message was sent while issue was closed.
Description was changed from ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 ========== to ========== Templatize SkToXXX. Makes the checked cast in debug more correct, avoiding new warnings in vs2015. BUG=skia:4553 Committed: https://skia.googlesource.com/skia/+/0be9e806af72b3e029e691eef5c891c90d3fd320 Committed: https://skia.googlesource.com/skia/+/68c14d9b3244ef9d58727ff11f2742be9236f46e ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://skia.googlesource.com/skia/+/68c14d9b3244ef9d58727ff11f2742be9236f46e |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
