Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2161103002:
Improve assert reporting. (Closed)
|
Created:
4 years, 5 months ago by bungeman-skia Modified:
4 years, 5 months ago Reviewers:
djsollen CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionImprove assert reporting.
Make SkASSERTF output readable.
Ensure the assert predicate is stringified once.
Make the abort code consistent.
TBR=reed
This doesn't change any public API, most of this should be privatized.
Committed: https://skia.googlesource.com/skia/+/1f790aaeef47b02fe72b9b62d1e8c7ead85ae442
Patch Set 1 #Patch Set 2 : Make action more consistent, check output. #Patch Set 3 : Remove test crashes. #Patch Set 4 : No stuttering. #Patch Set 5 : Consistent names. #
Total comments: 2
Patch Set 6 : Address comments. #
Messages
Total messages: 30 (24 generated)
Description was changed from ========== Improve assert reporting. ========== to ========== Improve assert reporting. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2161103002 ==========
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/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Improve assert reporting. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2161103002 ========== to ========== Improve assert reporting. Make SkASSERTF output readable and ensure the assert predicate is stringified once instead of multiple times. ==========
Description was changed from ========== Improve assert reporting. Make SkASSERTF output readable and ensure the assert predicate is stringified once instead of multiple times. ========== to ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. ==========
Description was changed from ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. ========== to ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2161103002 ==========
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/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2161103002 ========== to ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. ==========
Description was changed from ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. ========== to ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2161103002 ==========
Description was changed from ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2161103002 ========== to ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. ==========
bungeman@google.com changed reviewers: + djsollen@google.com
Lat one to touch it gets to review it.
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/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2161103002/diff/80001/include/core/SkTypes.h File include/core/SkTypes.h (right): https://codereview.chromium.org/2161103002/diff/80001/include/core/SkTypes.h#... include/core/SkTypes.h:151: #define SkDEBUGFAILF(fmt, ...) SkDebugf(fmt"\n", __VA_ARGS__); SK_ABORT("not reached") why change this last one?
Description was changed from ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. ========== to ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2161103002 ==========
The CQ bit was checked by bungeman@google.com to run a CQ dry run
https://codereview.chromium.org/2161103002/diff/80001/include/core/SkTypes.h File include/core/SkTypes.h (right): https://codereview.chromium.org/2161103002/diff/80001/include/core/SkTypes.h#... include/core/SkTypes.h:151: #define SkDEBUGFAILF(fmt, ...) SkDebugf(fmt"\n", __VA_ARGS__); SK_ABORT("not reached") On 2016/07/20 15:01:25, djsollen wrote: > why change this last one? Because I wanted the error message to state "not reached" instead of "assert(false)". Though maybe this is insignificant. Also, at some point while I was writing this CL it didn't make sense at some point. SK_ABORT could be SK_ABORT(fmt, ...) and then we wouldn't have to split this up. Ideally these would all be functions, they are macros just so SK_ABORT can get at __FILE__ and __LINE__ . Done. In other words, what I really want to write here is #define SkDEBUGFAILF(fmt, ...) SK_ABORT(fmt, __VA_ARGS__) but that's another CL, so I'll just make this SkASSERTF(false, fmt, ...) for now.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
Description was changed from ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2161103002 ========== to ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. TBR=reed This doesn't change any public API, most of this should be privatized. ==========
The CQ bit was checked by bungeman@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. TBR=reed This doesn't change any public API, most of this should be privatized. ========== to ========== Improve assert reporting. Make SkASSERTF output readable. Ensure the assert predicate is stringified once. Make the abort code consistent. TBR=reed This doesn't change any public API, most of this should be privatized. Committed: https://skia.googlesource.com/skia/+/1f790aaeef47b02fe72b9b62d1e8c7ead85ae442 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://skia.googlesource.com/skia/+/1f790aaeef47b02fe72b9b62d1e8c7ead85ae442 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
