[PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

[Alexander Shaposhnikov via cfe-commits]

alexshap added a comment.

disclaimer: i don't know if this method is the right thing to add (to be honest i would prefer a better interface but don't have any good suggestions on my mind at the moment), i see several issues (IMO, i might be mistaken, apologize in advance) with the current interface of Replacements. I will not list all of them, but want to add some context:

A. right now i see the same code (with minor changes) in several places:

1.

llvm/tools/clang/tools/extra/include-fixer/IncludeFixer.cpp +382

  auto R = tooling::Replacement(
        {FilePath, Info.Range.getOffset(), Info.Range.getLength(),
        Context.getHeaderInfos().front().QualifiedName});
   auto Err = Replaces.add(R);
   if (Err) {
     llvm::consumeError(std::move(Err));
     R = tooling::Replacement(
         R.getFilePath(), Replaces.getShiftedCodePosition(R.getOffset()),
         R.getLength(), R.getReplacementText());
     Replaces = Replaces.merge(tooling::Replacements(R));
   }

2.

llvm/tools/clang/tools/extra/change-namespace/ChangeNamespace.cpp +126 (see https://reviews.llvm.org/D24183)

   
  void addOrMergeReplacement(const tooling::Replacement &R,
                           tooling::Replacements *Replaces) {
   auto Err = Replaces->add(R);
   if (Err) {
      llvm::consumeError(std::move(Err));
      auto Replace = getReplacementInChangedCode(*Replaces, R);
      *Replaces = Replaces->merge(tooling::Replacements(Replace));
   }
  }

B. For replacements in headers we can get into if(Err) branch quite often because the same replacement can be generated multiple times (if that header is included into several *.cpp files).


https://reviews.llvm.org/D24383