[libc-commits] [PATCH] D78585: [libc][NFC] Cleanup dependencies in src/signal and test/src/signal.

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Tue Apr 21 22:40:45 PDT 2020


sivachandra marked 3 inline comments as done.
sivachandra added inline comments.


================
Comment at: libc/test/src/signal/CMakeLists.txt:24
     libc.src.signal.sigaction
     libc.src.signal.raise
 )
----------------
abrachet wrote:
> Nit: we could put raise above sigaction like we do everywhere else.
> 
> Not for this patch, but It would be nice if all of our dependencies were alphabetical. I've not done a good job of that. I will look into if there is a programmatic way to have them ordered nicely.
Fixed this in the commit. 

If you end up with something like a Python script which can formally parse our CMakeLists.txt files, that will be awesome and we can put it in say `utils/buildutils`.


================
Comment at: libc/test/src/signal/CMakeLists.txt:35
+    libc.include.errno
     libc.src.errno.__errno_location
     libc.src.signal.raise
----------------
abrachet wrote:
> Probably I should remove the line https://github.com/llvm/llvm-project/blob/master/libc/test/src/signal/sigprocmask_test.cpp#L36 and remove this too. But maybe not appropriate for this patch.
Ack.


================
Comment at: libc/test/src/signal/CMakeLists.txt:63
     libc.include.signal
     libc.src.errno.__errno_location
     libc.src.signal.raise
----------------
abrachet wrote:
> Ditto here and https://github.com/llvm/llvm-project/blob/master/libc/test/src/signal/signal_test.cpp#L22
Ack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78585





More information about the libc-commits mailing list