sammccall added subscribers: chandlerc, sammccall.
sammccall added inline comments.


================
Comment at: 
lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:77
+#define LLDB_PROPERTIES_dynamicloaderdarwinkernel
+#include "Properties.inc"
+};
----------------
These unqualified `#include` directives are causing us some problems 
downstream. What do you think about qualifying these like other includes in the 
file? i.e. `#include "Plugins/DynamicLoader/Darwin-Kernel/Properties.inc"`

The problem is that these "local" includes can only resolve to the right thing 
if the build system specifies a different `-I` line to each relevant cpp file 
(or the header lives next to the source file, which isn't the case for 
generated files).
Many build systems (including both CMake and bazel) only support doing this for 
separate libraries, however lldb's source isn't sufficiently clearly layered to 
allow splitting into fine-grained libraries in bazel (CMake is much more lax 
here).

While CMake is the gold standard here, it'd be helpful to make it possible to 
build with other systems out-of-tree. @chandlerc was looking at this issue and 
has more context and opinions here.

It also seems a bit less magic and more consistent with the rest of the 
includes in the file, as well as with the project. (Most, though sadly not all, 
.inc files in llvm are included by qualified paths).

I'll send a patch shortly after verifying it fixes the issue.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65185



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to