This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Remove redundant calls to set eReturnStatusFailed
ClosedPublic

Authored by DavidSpickett on Jun 17 2021, 4:53 AM.

Details

Summary

This is part 2, covering the commands source.

Some uses remain where it's tricky to see what the
logic is or they are not used with AppendError.

Diff Detail

Event Timeline

DavidSpickett requested review of this revision.Jun 17 2021, 4:53 AM
DavidSpickett created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2021, 4:53 AM
teemperor requested changes to this revision.Jun 17 2021, 5:05 AM

There are two places where this isn't redundant (see inline comments) but otherwise this LGTM. Thanks!

lldb/source/Commands/CommandObjectWatchpoint.cpp
892

AppendErrorWithFormat otherwise the status isn't changed.

915

Same as above.

This revision now requires changes to proceed.Jun 17 2021, 5:05 AM

Convert GetErrorStream().Printf to AppendError in CommandObjectWatchpoint.cpp.

(there's a bunch of other uses like that that I intend to modernise in a part 3)

DavidSpickett marked 2 inline comments as done.
DavidSpickett added a reviewer: teemperor.
teemperor accepted this revision.Jun 17 2021, 6:28 AM

LGTM now, just a small nit about the redundant "error: " string.

lldb/source/Commands/CommandObjectWatchpoint.cpp
879–880

AppendError adds the error: prefix so it shouldn't be in the passed message

900

here too

This revision is now accepted and ready to land.Jun 17 2021, 6:28 AM

Good catch, the newline is also not needed.

DavidSpickett marked 2 inline comments as done.Jun 17 2021, 6:37 AM
This revision was landed with ongoing or failed builds.Jun 17 2021, 6:39 AM
This revision was automatically updated to reflect the committed changes.