[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

Sam Elliott via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 11 09:26:49 PST 2022


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



More information about the cfe-commits mailing list