[PATCH] D76885: [lld][COFF][ELF][WebAssembly] Replace --[no-]threads /threads[:no] with --threads={1,2,...} /threads:{1,2,...}

[dmajor via Phabricator via llvm-commits]

dmajor added a comment.
Herald added a subscriber: dang.

> --no-threads is used rarely. So just delete --no-threads instead of keeping it for compatibility for a while.

I encountered a frustrating issue with seemingly-unrelated symptoms that eventually traced back to this change.

The wasi-sdk project was building libcxx, and its build system had set `CMAKE_EXE_LINKER_FLAGS="-Wl,--no-threads"` (I don't know why, wasi-sdk is an unfamiliar project to me, I just needed to build it for something else). When cmake was doing its usual "is XYZ supported" tests, the `--no-threads` option caused the checks to fail, so cmake believed that for example `LIBCXX_SUPPORTS_FNO_EXCEPTIONS_FLAG` was false. But the cmake was overall still successful. So we got an apparently successful libcxx build but it didn't have the settings that were requested. This manifested as weird issues in a faraway place later. It took a lot of time to track down.

It's not clear to me why the `--no-threads` made the "is XYZ supported" checks fail but the actual build still succeeded. I'm pretty worried though about introducing this kind of silent behavior change. Do you think other projects may be susceptible to such a problem? Should the lack of `--no-threads` compatibility be reconsidered for the 11 release branch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76885