serge-sans-paille added a comment. Hi and thanks for taking the time to go through the review.
In D118652#3285601 <https://reviews.llvm.org/D118652#3285601>, @MaskRay wrote: > Thanks for working on this. I spot checked some files. In general it looks > great. > > One file replaces a forward declaration with `#include > "llvm/IR/BasicBlock.h"`. Yeah. What happens is that header was previously implicitly included (and needed), I just made the include explicit (and the forward declaration useless) > Some files add `#include "llvm/Support/ToolOutputFile.h"`. Yeah, that's a consequence of `llvm/IR/LLVMRemarkStreamer.h no longer includes llvm/Support/ToolOutputFile.h` > One files adds `class FunctionPass;` Yeah, that happens when the file was implicitly including `llvm/Pass.h` but was actually only needing a forward declaration. > Would you mind landing these addition changes separately to make the include > removal part more targeted? The three changes above are tied to individual header removal in specific header. I could split that in smaller grain (say each header change + its impact on the codebase in individual commits), but that would mean that every "component" cleanup would consist in potentially dozens of commits. I understand the value of such fine-grain commits, but it means more care (and work!) on my side to handle more reviews, craft more decent commit message etc. I chose to split the cleanup per component to make review easier, have a decent idea of the impact on preprocessed lines. I'm trying to reflect in the commit message the major changes to help downstream users update their codebase. If you really think the split would help the review process, I can do it, but please take into account the amount of work it adds on an already not super-creative task ;-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118652/new/ https://reviews.llvm.org/D118652 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
