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