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

[Guillaume Chatelet via Phabricator via libc-commits]

gchatelet 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()
+
----------------
sivachandra wrote:
> gchatelet wrote:
> > sivachandra wrote:
> > > 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.
> > I initially did the two step version with `FORCE` to make sure that `CMake` does not retain an old value.
> > 
> > i.e., if we run `cmake -DLIBC_NAMESPACE=__llvm_libc ...` then `cmake ...` I would expect the second run to use the autogenerated namespace value. WDYT?
> > I initially did the two step version with `FORCE` to make sure that `CMake` does not retain an old value.
> 
> The way I understand it, the `FORCE` on line 52 just clears out the command line arg also?
> 
> > i.e., if we run `cmake -DLIBC_NAMESPACE=__llvm_libc ...` then `cmake ...` I would expect the second run to use the autogenerated namespace value. WDYT?
> 
> I don't think this is possible in general at all? As in, I am not sure there is a way to overwrite cache variables that way at all other than `FORCE` of course, but `FORCE` erases the command line value the first time as well?
> 
> If one is making another CMake run with a different set of command line options, they should cleanup the files from the previous run.  I went looking for an option and found the `--fresh` option but it is available only from 3.24: https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-fresh.
Alright, let's make it simple then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159112