[Lldb-commits] [PATCH] D140056: [lldb] Report clang module build remarks

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.


================
Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:82
   Log *m_log;
+  std::unique_ptr<Progress> m_current_progress_up;
+  std::vector<std::string> m_module_build_stack;
----------------
aprantl wrote:
> kastiglione wrote:
> > mib wrote:
> > > `Progress` makes use of RAII to complete the progress report event.
> > > 
> > > I think a local variable would be more suitable.
> > Since this is a callback, and the work being done doesn't happen in this function, I reasoned that that a local wouldn't match the semantics. Since this function is so short lived, the Progress would be immediately destructed and completed, but the work of building the module is not yet completed. Also, when an event is completed, the console text is cleared. As a result, the user doesn't see which module is currently building.
> Might be good to comment this?
by extending the life of the Progress, this code, which clears the console of the current progress message, is deferred.

https://github.com/apple/llvm-project/blob/d0fbbd7e14e13b0f915eefdb20b57a60eb177039/lldb/source/Core/Debugger.cpp#L1945-L1949

```
  if (data->GetCompleted() == data->GetTotal()) {
    // Clear the current line.
    output->Printf("\x1B[2K");
    output->Flush();
    return;
```

This screen clearing deferred until either a new remark_module_build is emitted, or a remark_module_build_done is emitted. This allows the current message to stay on for screen, showing the user what's being built and giving them a sense for how long it's taking.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140056/new/

https://reviews.llvm.org/D140056