[libc-commits] [PATCH] D159112: [libc] POC for namespace customization

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/CMakeLists.txt:58
+  set(LLVM_LIBC_NAMESPACE "__llvm_libc_${LLVM_VERSION_MAJOR}_${LLVM_VERSION_MINOR}_${LLVM_VERSION_PATCH}_${LLVM_VERSION_SUFFIX}")
+endif()
+
----------------
Can we replace lines 51 to 58 with:

```
set(LIBC_NAMESPACE
  "__llvm_libc_${LLVM_VERSION_MAJOR}_${LLVM_VERSION_MINOR}_${LLVM_VERSION_PATCH}_${LLVM_VERSION_SUFFIX}"
  CACHE STRING "The namespace to use to enclose internal implementations. Must start with '__llvm_libc'."
)
```

The effective changes are:

1. Call the CMake variable just `LIBC_NAMESPACE`.
2. Since it is a `CACHE` var, it will be set only if it is not being set on the command line.
3. We don't need `FORCE` as we do not really want to change the cached value.


================
Comment at: libc/cmake/modules/LLVMLibCCompileOptions.cmake:10
+        target_compile_options(${target_name} PRIVATE "/DLIBC_NAMESPACE=${LLVM_LIBC_NAMESPACE}")
+    endif()
+endfunction()
----------------
Instead of adding a compile option to every target, can we just do one [[ https://cmake.org/cmake/help/latest/command/add_definitions.html | add_definitions ]] call in `libc/CMakeLists.txt`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159112