[Openmp-commits] [PATCH] D100185: [OpenMP] [OMPD] [5/6] Implementation of OMPD debugging library - libompd. gdb-plugin: A Plugin code to gdb to leverage libompd to provide debugging support.

[Aaron Puchert via Phabricator via Openmp-commits]

aaronpuchert added inline comments.


================
Comment at: openmp/libompd/CMakeLists.txt:14
     add_subdirectory(src)
+    if(NOT DISABLE_OMPD_GDB_PLUGIN)
+        add_subdirectory(gdb-plugin)
----------------
Could you introduce this as cache variable? Something like
```
set(DISABLE_OMPD_GDB_PLUGIN OFF CACHE BOOL "<Some description>")
```
See also https://cmake.org/cmake/help/latest/command/set.html#set-cache-entry.

And perhaps we could choose the default based on the availability of Python 3 development files?


================
Comment at: openmp/libompd/gdb-plugin/CMakeLists.txt:17
+find_package (Python3 COMPONENTS Interpreter Development)
+find_package (PythonLibs REQUIRED)
+
----------------
Isn't this the same as the `Development` component from the previous line? Down there I don't see direct usage of `PythonLibs`, only `Python3_INCLUDE_DIRS` and `Python3_LIBRARIES` from the previous line.

[CMake documentation](https://cmake.org/cmake/help/latest/module/FindPythonLibs.html) also says that this is deprecated in favor of `Python3` that you're already using. So this line can likely simply be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100185