h-vetinari added inline comments.

================
Comment at: libunwind/src/CMakeLists.txt:28-35
 
 # See add_asm_sources() in compiler-rt for explanation of this workaround.
 # CMake doesn't work correctly with assembly on AIX. Workaround by compiling
 # as C files as well.
 if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR
-   (MINGW AND CMAKE_VERSION VERSION_LESS 3.17) OR
-   (${CMAKE_SYSTEM_NAME} MATCHES "AIX"))
+   (MINGW AND CMAKE_VERSION VERSION_LESS 3.17))
   set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)
----------------
mstorsjo wrote:
> h-vetinari wrote:
> > Shouldn't it be possible to remove that entire block (as it only fires for 
> > CMake <3.20)?
> The intention was to do such cleanups in a later patch, as mentioned in the 
> original commit message in https://reviews.llvm.org/D144509. (I guess it 
> would be good to bring the original commit message along with the reland 
> attempts too.)
Well sure, but "workarounds" is a broad term. It makes sense to not try to 
clean up everything that newer CMake enables, but this PR does remove a lot of 
(all?) other lines with `CMAKE_VERSION VERSION_LESS <x>` where x<3.20. On top 
of that, since this line is already being changed (which is why I noticed in 
the first place), I'd suggest to just remove it. But I don't feel strongly 
about this, just thought I'd point it out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151344

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to