JDevlieghere added a comment.

This is in line with the existing ways of sourcing init files in LLDB and has 
all the things we discussed in the RFC. I left few nits but this all looks good 
to me. I'll hold off on accepting so it continues to show up in the other 
reviewer's queue.



================
Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:256
   void SourceInitFileHome(CommandReturnObject &result, bool is_repl);
+  void SourceSystemInitFile(CommandReturnObject &result);
 
----------------
`SourceInitFileSystem` for consistency with the other two? Or maybe you were 
planning to change the other two in another patch?


================
Comment at: lldb/source/API/SBCommandInterpreter.cpp:420-422
+void SBCommandInterpreter::SourceSystemInitFile(SBCommandReturnObject &result) 
{
+  LLDB_INSTRUMENT_VA(this, result);
+  result.Clear();
----------------
Nit: all the other functions //should// have a newline because that's what 
`lldb-instr` generates.


================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2385-2393
+#ifdef LLDB_SYSTEM_INIT_PATH
+    FileSpec init_file(LLDB_SYSTEM_INIT_PATH);
+    if (init_file) {
+      init_file.MakeAbsolute(HostInfo::GetShlibDir());
+    }
+
+    SourceInitFile(init_file, result);
----------------
Why not put the ifdef around the `m_skip_lldbinit_files` check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119831

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to