[Lldb-commits] [PATCH] D115324: Added the ability to cache the finalized symbol tables subsequent debug sessions to start faster.

[Greg Clayton via Phabricator via lldb-commits]

clayborg marked 6 inline comments as done.
clayborg added a comment.

Pavel: I fixed all issues you identified. Let me know if there is anything else you would like to see changed.



================
Comment at: lldb/source/Host/common/FileSystem.cpp:523
+  Status error;
+  if (IsDirectory(path))
+    error.SetErrorStringWithFormatv("\"{0}\" is a directory",
----------------
labath wrote:
> What's the purpose of this check ?
I checked the llvm code, and it already correctly handles not deleting a file if it is a directory, so this isn't needed.


================
Comment at: lldb/source/Symbol/Symbol.cpp:645
+  file.AppendU16(m_type_data);
+  file.AppendU16((&m_type_data)[1]);
+  m_mangled.Encode(file, strtab);
----------------
labath wrote:
> This is quite dodgy. It would be better to change the storage so that we don't have to do this. Perhaps something like
> ```
> union {
>   uint16_t flag_storage;
>   struct {
>     uint16_t m_type_data_resolved : 1;
>     ...
>   };
> };
> ```
I encoded them manually now. It was too disruptive to the code to try and add an anonymous union with an anonymous struct as there were many compiler warnings about it being a GNU extension and made the code way too messy, especially in the constructor as you can't init each value easily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115324