JDevlieghere marked an inline comment as done. JDevlieghere added inline comments.
================ Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4300-4301 + current_frame_flush)) + symbols_found = true; + flush |= current_frame_flush; + } ---------------- kastiglione wrote: > JDevlieghere wrote: > > kastiglione wrote: > > > do you need the separate variable? can it be: > > > > > > ``` > > > flush |= true; > > > ``` > > I want `flush` to be true only if at least one frame requires a flush. > my suggestion still achieves that. > > Instead of: > > ``` > bool current_frame_flush = false; > if (DownloadObjectAndSymbolFile(module_spec, target, result, > current_frame_flush)) > symbols_found = true; > > flush |= current_frame_flush; > ``` > > Inline the `current_frame_flush` variable: > > ``` > if (DownloadObjectAndSymbolFile(module_spec, target, result, > current_frame_flush)) > flush |= true; > ``` > > This makes `flush` only true when one frame needs it. That relies on the assumption that flush is true iff `DownloadObjectAndSymbolFile`return true (and the same for `AddModuleSymbols`). Looking at the code that does indeed seem to be the case, but then I'd rather see the API refactored to remove the `flush` output variable instead of relying on an implementation-specific assumption here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110011/new/ https://reviews.llvm.org/D110011 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
