[libc-commits] [PATCH] D101991: [libc] Allow target architecture customization

[Guillaume Chatelet via Phabricator via libc-commits]

gchatelet added inline comments.


================
Comment at: libc/cmake/modules/LLVMLibCObjectRules.cmake:44
   )
-  if(ADD_OBJECT_COMPILE_OPTIONS)
-    target_compile_options(
-      ${fq_target_name}
-      PRIVATE ${ADD_OBJECT_COMPILE_OPTIONS}
-    )
-  endif()
+  _set_common_compile_options(${ADD_OBJECT_COMPILE_OPTIONS})
+  target_compile_options(${fq_target_name} PRIVATE ${LIBC_COMMON_COMPILE_OPTIONS})
----------------
sivachandra wrote:
> Instead of `_set_*`, I think a function with name `_get_*` will be more appropriate here. The function should also take an additional arg for the result:
> 
> ```
> _get_all_compile_options(all_opts ...)
> target_compile_options(${fq_target_name} PRIVATE ${all_opts})
> ```
> 
> This probably a nitty comment but motivated by the fact that `common` is not suitable in `add_object_library`.
I was pondering whether get or set was more suitable and I've been lazy : )
Thx for the comment it makes sense to go with get indeed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101991