[libcxx-commits] [PATCH] D124377: [runtimes] [CMake] Fix checks for -Werror when building with incomplete toolchains

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: runtimes/CMakeLists.txt:106-124
     set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
+    # TODO: When we can require CMake 3.14, we should use
+    # CMAKE_REQUIRED_LINK_OPTIONS here. Until then, we need a workaround:
+    # When using CMAKE_REQUIRED_FLAGS, this option gets added both to
+    # compilation and linking commands. That causes warnings in the
+    # compilation commands during cmake tests. This is normally benign, but
+    # when testing whether -Werror works, that test fails (due to the
----------------
Shouldn't this be more like

```
if (C_SUPPORTS_START_NO_UNUSED_ARGUMENTS)
  set(CMAKE_REQUIRED_FLAGS "${ORIG_CMAKE_REQUIRED_FLAGS} --start-no-unused-arguments --unwindlib=none --end-no-unused-arguments")
else()
  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
endif()
```

Right now you seem to be executing

```
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
```

in all cases.


**Edit** Oh, I see it now. You override `CMAKE_REQUIRED_FLAGS` with `ORIG_CMAKE_REQUIRED_FLAGS` when `C_SUPPORTS_START_NO_UNUSED_ARGUMENTS` is supported. I think it would be clearer to use a if-else instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124377