[Lldb-commits] [PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

arichardson added inline comments. Comment at: llvm/CMakeLists.txt:5 +include(GNUInstallDirs) + This seems to be causing the following warning for me: ``` CMake Warning (dev) at /opt/clion-2021.2/bin/cmake/linux/share/cmake-3.20/Modules/GNUInstallDirs.cmake:2

[Lldb-commits] [PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

arichardson added inline comments. Comment at: llvm/CMakeLists.txt:5 +include(GNUInstallDirs) + arichardson wrote: > This seems to be causing the following warning for me: > > ``` > CMake Warning (dev) at > /opt/clion-2021.2/bin/cmake/linux/share/cmake-3.20/M

[Lldb-commits] [PATCH] D117639: [cmake] Avoid warning or extra suffix for CMAKE_INSTALL_LIBDIR

arichardson added a comment. I can't see anywhere where the GNUInstallDirs variables are used before project(), so can't we just move the includes further down? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117639/new/ https://reviews.llvm.org/D11

[Lldb-commits] [PATCH] D117639: [cmake] Avoid warning or extra suffix for CMAKE_INSTALL_LIBDIR

arichardson added a comment. I believe `LLVM_LIBDIR_SUFFIX` was added as a workaround for not being able to use GNUInstallDirs (which will automatically detect the right suffixed e.g. for SuSE) in rG46fed3b475ddd92d02d9b72d0d77c5a939f132d1

[Lldb-commits] [PATCH] D117639: [cmake] Avoid warning or extra suffix for CMAKE_INSTALL_LIBDIR

arichardson added a comment. I see AddLLVM already includes GNUInstallDirs, can we add the LLVM_LIBDIR_SUFFIX check to that file and avoid including GNUInstallDirs explicitly in all the projects? The standalone builds will pull in GNUInstallDirs via AddLLVM, and the non-standalone builds should

[Lldb-commits] [PATCH] D117639: [cmake] Make include(GNUInstallDirs) always below project(..)

arichardson accepted this revision. arichardson added a comment. This LGTM, but please wait for someone else to review before committing it. Comment at: lldb/CMakeLists.txt:21 +if(LLDB_BUILT_STANDALONE) include(LLDBStandalone) Could keep this in the first

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

arichardson added inline comments. Comment at: cmake/Modules/GNUBinaryDirs.cmake:6 +if (NOT DEFINED CMAKE_BINARY_BINDIR) + set(CMAKE_BINARY_BINDIR "${CMAKE_BINARY_BINDIR}/bin") +endif() I find this name a bit confusing, maybe something like `CMAKE_BUILD_BINDIR`

[Lldb-commits] [PATCH] D141219: Add a .lldbinit file to autoload LLVM/Clang data formatters

arichardson created this revision. arichardson added a reviewer: LLDB. Herald added a project: All. arichardson requested review of this revision. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. This makes it much easier to debug LLVM/Clang when using an IDE such as CLion th

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

arichardson added a comment. Minor suggestions, feel free to ignore. Comment at: lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp:14 FreeBSDSignals::FreeBSDSignals() : UnixSignals() { Reset(); } void FreeBSDSignals::Reset() { I wonder if it would make

[Lldb-commits] [PATCH] D146222: [lldb] Add compile time checks for signal codes when on the matching platform

arichardson accepted this revision. arichardson added a comment. This revision is now accepted and ready to land. Much neater than my initial suggestion. LGTM Comment at: lldb/source/Plugins/Process/Utility/LinuxSignals.cpp:14 + +#ifndef SEGV_BNDERR +#define SEGV_BNDERR 3 -

[Lldb-commits] [PATCH] D146222: [lldb] Add compile time checks for signal codes when on the matching platform

arichardson added inline comments. Comment at: lldb/source/Plugins/Process/Utility/NetBSDSignals.cpp:15 +#ifndef SEGV_BNDERR +#define SEGV_BNDERR 3 +#endif Also unused like for FreeBSD? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[Lldb-commits] [PATCH] D88796: [lldb] Initial version of FreeBSD remote process plugin

arichardson added inline comments. Comment at: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp:492 + } + for (i = 0; i < count; i++) { +MemoryRegionInfo info; This code is C++ so using for-loop initializers is fine. CHANGES SINCE LAST

[Lldb-commits] [PATCH] D66795: [Mips] Use appropriate private label prefix based on Mips ABI

arichardson accepted this revision. arichardson added a comment. This revision is now accepted and ready to land. Looks good to me but I guess someone else should give the final approval. Comment at: llvm/lib/Target/Mips/MCTargetDesc/MipsMCAsmInfo.cpp:25 if (TheTriple.isMI