Page MenuHomePhabricator

LanguageRuntime: Simplify NSException::GetSummary() output
ClosedPublic

Authored by mib on Dec 10 2019, 3:37 PM.

Details

Summary

Right now, NSException::GetSummary() has the following output:
"name: $exception_name - reason: $exception_reason"

It would be better to simplify the output by removing the name and only
showing the exception's reason. This way, annotations would look nicer in
the editor, and would be a shorter summary in the Variables Inspector.

Accessing the exception's name can still be done by expanding the
NSException object in the Variables Inspector.

rdar://54770115

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

Diff Detail

Event Timeline

mib created this revision.Dec 10 2019, 3:37 PM
mib added a reviewer: Restricted Project.Dec 10 2019, 3:38 PM
JDevlieghere added inline comments.
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjNSException.py
29

Can we modify these strings so they're different for the different exceptions?

davide added a subscriber: davide.Dec 10 2019, 3:53 PM

This one looks fine -- can you please check that this patch applied on swift still produces something reasonable for mixed obj-C/swift formatters?
My guess is that it will, and you probably just need to update some tests.

jingham added inline comments.
lldb/source/Plugins/Language/ObjC/NSException.cpp
111–114

I wonder if it would be better to print "No reason" as the summary when there's not a reason string? Having the summary generally be there, but then not be there at all when the exception wasn't given a reason string, is a little disconcerting.

davide added inline comments.Dec 10 2019, 4:22 PM
lldb/source/Plugins/Language/ObjC/NSException.cpp
111–114

Arguably.

mib updated this revision to Diff 233239.Dec 10 2019, 5:19 PM

Changed tests exception reasons.
Added print for when there is not reason.

mib updated this revision to Diff 233240.Dec 10 2019, 5:22 PM

Fixed patch submission

Harbormaster completed remote builds in B42268: Diff 233240.
mib marked 3 inline comments as done.Dec 10 2019, 5:23 PM
JDevlieghere accepted this revision.Dec 11 2019, 10:07 AM
This revision is now accepted and ready to land.Dec 11 2019, 10:07 AM
davide requested changes to this revision.Dec 11 2019, 10:13 AM

Did you test on swift?

This revision now requires changes to proceed.Dec 11 2019, 10:13 AM
mib added a comment.Dec 13 2019, 11:17 AM

@davide I ran the Swift test suite both with and without my patch and got the same results :)

davide accepted this revision.Dec 13 2019, 11:28 AM

LGTM!

This revision is now accepted and ready to land.Dec 13 2019, 11:28 AM
This revision was automatically updated to reflect the committed changes.