lenary added a comment. In D137838#3921828 <https://reviews.llvm.org/D137838#3921828>, @thakis wrote:
> This is a gigantic diff. I'd recommend keeping the .h files in the old place > for v0 and make them just forwarding headers that include the header at the > new location. That way, you don't have to update oodles of include lines in > this patch, and it makes it a bit easier to see what's going on. (You can > then update all the include lines in a trivial follow-up if this change goes > through, and then remove the forwarding headers in Support, to cut the > dependency you want to remove.) Thanks for the suggestion. I do agree this patch is too big to land as-is. I think this patch is useful as "this shows the full effect of this change", even if we find ways to make how we land this patch less invasive. One other thought I had was to move the unittests first, but that doesn't make as big a difference as the fact that `llvm/ADT/Triple.h` is referenced everywhere. I would like more comments (either here or on Discourse, link in a prior comment) on whether this split is reasonable before I post a v2 of this patch, though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org/D137838 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
