[PATCH] D80239: [libTooling] In Transformer, allow atomic changes to span multiple files.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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