JDevlieghere added a subscriber: dexonsmith.
JDevlieghere added a comment.
I looked at all the call sites and they all seemed reasonable in terms of doing
work on behalf of the user or not and selecting the most relevant frame
respectively. My only concern is that we now have a bunch of places where we're
passing a blind `true` or `false`. While it's pretty obvious in this patch, it
won't be so obvious when you're reading the same code in the future, let alone
when someone inevitably copy-pastes it. I can think of two easy ways to improve
readability:
1. Add an inline comment:
StackFrameSP frame_sp = GetSelectedFrame(/*select_most_relevant=*/false);
2. Add an enum to `Frame`:
enum SelectMostRelevant : bool {
SelectMostRelevantFrame = true,
DoNoSelectMostRelevantFrame = false,
};
StackFrameSP frame_sp = GetSelectedFrame(SelectMostRelevantFrame);
This is a trick that @dexonsmith thought me a long time ago. I personally like
it the best because it makes things very explicit without the hassle of having
to actually treat the value like an enum.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148863/new/
https://reviews.llvm.org/D148863
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
