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

Alexander Shaposhnikov via cfe-commits cfe-commits at lists.llvm.org
Sat Sep 10 01:54:37 PDT 2016

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:


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

  auto R = tooling::Replacement(
        {FilePath, Info.Range.getOffset(), Info.Range.getLength(),
   auto Err = Replaces.add(R);
   if (Err) {
     R = tooling::Replacement(
         R.getFilePath(), Replaces.getShiftedCodePosition(R.getOffset()),
         R.getLength(), R.getReplacementText());
     Replaces = Replaces.merge(tooling::Replacements(R));


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) {
      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).


More information about the cfe-commits mailing list