[PATCH] D114664: [objcopy][NFC] Add rules to cmake to put files under specific folders.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Object/CMakeLists.txt:1
+
+source_group("Header Files\\ObjCopy" FILES
----------------
Nit: delete this blank line.


================
Comment at: llvm/lib/Object/CMakeLists.txt:2
+
+source_group("Header Files\\ObjCopy" FILES
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Object/ObjCopy/CommonConfig.h
----------------
The backslashes are surprising to me. Can this be forward slashes, like a cross-platform file path?


================
Comment at: llvm/lib/Object/CMakeLists.txt:3
+source_group("Header Files\\ObjCopy" FILES
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Object/ObjCopy/CommonConfig.h
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Object/ObjCopy/ConfigManager.h
----------------
I think you want to use the `REGULAR_EXPRESSION` rather than `FILES`, so that you don't need to explicitly specify each source file in the list.

This should allow you to avoid the need for the macros below too.


================
Comment at: llvm/lib/Object/CMakeLists.txt:107-110
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Object/ObjCopy/COFF
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Object/ObjCopy/ELF
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Object/ObjCopy/MachO
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Object/ObjCopy/wasm
----------------
Do you need these explicitly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114664