mstorsjo added inline comments.
================ Comment at: lldb/tools/lldb-vscode/FifoFiles.cpp:9 + +#if !defined(WIN32) +#include <sys/stat.h> ---------------- stella.stamenova wrote: > Also, this is no good. It works if you are targeting windows on windows, but > not if you are targeting something else and building on windows. There are a > few different ways this is done in LLVM/clang and they are generally not > based on defined(WIN32). Here are a couple of examples: > > From llvm\lib\Support\Path.cpp: > > ``` > #if !defined(_MSC_VER) && !defined(__MINGW32__) > ``` > > From clang\lib\Driver\Driver.cpp: > > ``` > #if LLVM_ON_UNIX > ``` > > From llvm\lib\Support\ErrorHandling.cpp: > > ``` > #if defined(HAVE_UNISTD_H) > ``` > > I suggest browsing through the code and finding the most appropriate way to > manage your includes. In the mean time, this is breaking our internal builds > that run on Windows but do not target Windows. @stella.stamenova `defined(WIN32)` shouldn't be used (as that isn't predefined by the compiler), but `defined(_WIN32)` should be just fine (and is used a lot within LLDB already, 176 occurrances). If cross compiling on windows, targeting another os, neither of them should be defined, so it should be just fine, no? Or is this a case of cygwin, where the lines are blurred even more? Does the current main branch work fine for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93951/new/ https://reviews.llvm.org/D93951 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
