jobnoorman added a comment. In D150549#4351614 <https://reviews.llvm.org/D150549#4351614>, @MaskRay wrote:
> In D150549#4349531 <https://reviews.llvm.org/D150549#4349531>, @jobnoorman > wrote: > >> In D150549#4347056 <https://reviews.llvm.org/D150549#4347056>, @MaskRay >> wrote: >> >>> This seems fine, but please consider making `MC/SubtargetFeature.h` a >>> forwarding header temporarily. >> >> To be clear: do you want me to check-in this forwarding header or should I >> just do it locally for the tests below? If the former, what exactly is the >> purpose of adding this header? > > The motivation is similar to https://reviews.llvm.org/D137838#3921828 > >> 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.) > > (There is a non-zero probability that the patch may miss something and get > reverted. By creating a forwarding header we don't risk causing too much > churn to the many files.) > For a large-scaling refactoring Thanks for the link! If I understand the discussion there correctly, the motivation for adding the forwarding header was to not have to update any `#include` sites directly in order to reduce the size of the patch. So what you are asking me to do is 1. Update this patch to have a forwarding header and //not// change `#include` sites; 2. Create a second patch that updates `#include`s and removes the forward. This patch would be committed at some point in the future once we're sure the move is fine. Is this correct? I do wonder if this is necessary in this case as the patch is much smaller than the one in the link you shared. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150549/new/ https://reviews.llvm.org/D150549 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
