https://github.com/mstorsjo commented:
What test coverage do we have for targeting cygwin with clang right now? Do we have enough tests to make sure that this is a refactoring, so things work mostly like before, but with better structured code? If not, we should probably add some sort of minimal tests for this driver while adding it. FYI, as LLVM uses the "squash and merge" merge method, anything in the PR description ends up in the commit message (unless edited by whoever merges it, when merging), so for PRs it may be good to leave more discussion-like comments to a separate post after opening the PR. There was an earlier attempt to add a cygwin specific driver before, see https://github.com/llvm/llvm-project/pull/74933. That one didn't really go far though, but it seems to have much more code in the driver implementation. I guess this variant is simpler; where this is mostly a refactoring so far, making it easier to deal with. Then we can review incremental improvements on top of it. I see that that patch also contains some similar cleanups to InitHeaderSearch.cpp - do these patches share some common ancestry (e.g. some cygwin specific clang patchsets that have been floating around somewhere?). https://github.com/llvm/llvm-project/pull/135691 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
