ymandel marked 2 inline comments as done.
ymandel added a comment.
Thanks for the review.
================
Comment at: clang/lib/Tooling/Transformer/Transformer.cpp:65
- for (const auto &I : Case.AddedIncludes) {
- auto &Header = I.first;
- switch (I.second) {
- case transformer::IncludeFormat::Quoted:
- AC.addHeader(Header);
- break;
- case transformer::IncludeFormat::Angled:
- AC.addHeader((llvm::Twine("<") + Header + ">").str());
- break;
+ for (auto &IDChangePair : ChangesByFileID) {
+ auto &AC = IDChangePair.second;
----------------
gribozavr2 wrote:
> The test shows an example transformer that removes code, so the header
> insertion logic is not triggered there. However, for a change that would be
> adding code, is it correct to insert the header into every file being edited?
> I think not necessarily. Or do you prefer to deal with this issue when we
> have a sample use case?
You're right -- these two features don't mix well. Once we support multiple
files per transformation, we should change the header manipluation to be
change-specific rather than apply to the whole rule. That will require some
re-factoring of the APIs.
For now, I'll put in a FIXME, since this is not (yet) a high demand feature and
we'll just note the limitations.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80239/new/
https://reviews.llvm.org/D80239
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
