[Lldb-commits] [PATCH] D55584: [LLDB] Simplify Boolean expressions
Eugene.Zelenko added a comment. Thank you for cleanup effort! I would suggest to also run modernize checks and at least next readability checks: readability-container-size-empty readability-isolate-declaration readability-redundant-control-flow readability-redundant-member-init readability-redundant-smartptr-get readability-redundant-string-cstr readability-redundant-string-init Indeed, Clang-tidy offers even more useful checks. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55584/new/ https://reviews.llvm.org/D55584 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D35113: Clean up lldb-types.h
Eugene.Zelenko added inline comments. Comment at: include/lldb/Host/MainLoop.h:16 #include "llvm/ADT/DenseMap.h" +#include I think will be good idea to include csignal instead. Same in other files. See http://clang.llvm.org/extra/clang-tidy/checks/modernize-deprecated-headers.html. https://reviews.llvm.org/D35113 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)
Eugene.Zelenko added inline comments. Comment at: lldb/source/API/SBData.cpp:9 #include cinttypes should be used instead. See Clang-tidy modernize-deprecated-headers. Same in other places. Comment at: lldb/source/API/SBData.cpp:12 +#include + #include "lldb/API/SBData.h" Spaces between include statements interfere with Clang-format. Same in other places. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57990/new/ https://reviews.llvm.org/D57990 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)
Eugene.Zelenko added inline comments. Comment at: lldb/source/API/SBData.cpp:12 +#include + #include "lldb/API/SBData.h" JDevlieghere wrote: > jingham wrote: > > Eugene.Zelenko wrote: > > > Spaces between include statements interfere with Clang-format. Same in > > > other places. > > How? > > > > We always separate system header includes from lldb includes from llvm > > includes by putting a blank line. That's done all over the place in lldb. > > I haven't heard of this causing problems before now. If this does cause > > problems, that should be fixed in clang-format. It seems like a really > > unreasonable requirement. > Indeed, and as far as I know llvm does exactly the same thing. clang-format > sorts headers within the same group, which is exactly what we want. Is there any good reason to be different from LLVM/Clang? Wasn't formatting style was changed in not so recent past to reduce differences? Examples of problems visible in this patch: lldb/source/API/SBInstruction.cpp lldb/source/API/SBThread.cpp lldb/source/Commands/CommandObjectMemory.cpp lldb/source/Interpreter/CommandInterpreter.cpp Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57990/new/ https://reviews.llvm.org/D57990 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)
Eugene.Zelenko added inline comments. Comment at: lldb/source/API/SBData.cpp:12 +#include + #include "lldb/API/SBData.h" JDevlieghere wrote: > Eugene.Zelenko wrote: > > JDevlieghere wrote: > > > jingham wrote: > > > > Eugene.Zelenko wrote: > > > > > Spaces between include statements interfere with Clang-format. Same > > > > > in other places. > > > > How? > > > > > > > > We always separate system header includes from lldb includes from llvm > > > > includes by putting a blank line. That's done all over the place in > > > > lldb. I haven't heard of this causing problems before now. If this > > > > does cause problems, that should be fixed in clang-format. It seems > > > > like a really unreasonable requirement. > > > Indeed, and as far as I know llvm does exactly the same thing. > > > clang-format sorts headers within the same group, which is exactly what > > > we want. > > Is there any good reason to be different from LLVM/Clang? Wasn't formatting > > style was changed in not so recent past to reduce differences? > > > > Examples of problems visible in this patch: > > > > lldb/source/API/SBInstruction.cpp > > lldb/source/API/SBThread.cpp > > lldb/source/Commands/CommandObjectMemory.cpp > > lldb/source/Interpreter/CommandInterpreter.cpp > I'm saying it's exactly the same in LLDB as it is in LLVM. I'm afraid I don't > understand what you mean here. I don't remember empty lines between headers groups in LLVM and Clang code base. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57990/new/ https://reviews.llvm.org/D57990 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)
Eugene.Zelenko added inline comments.
Comment at: lldb/source/API/SBTypeEnumMember.cpp:78
TypeEnumMemberImpl &SBTypeEnumMember::ref() {
if (m_opaque_sp.get() == NULL)
+m_opaque_sp = std::make_shared();
tatyana-krasnukha wrote:
> ```
> if (!m_opaque_sp)
> ```
Will be good idea to run Clang-tidy readability-redundant-smartptr-get.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57990/new/
https://reviews.llvm.org/D57990
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)
Eugene.Zelenko added inline comments. Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:10 #include "ProcessMinidump.h" + #include "ThreadMinidump.h" Spurious separator. Comment at: lldb/source/Plugins/Process/minidump/ThreadMinidump.cpp:10 #include "ThreadMinidump.h" + #include "ProcessMinidump.h" Spurious separator. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:11 +#include + Wrong place. Comment at: lldb/tools/debugserver/source/debugserver.cpp:14 #include +#include #include Wrong place. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57990/new/ https://reviews.llvm.org/D57990 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
