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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
