[clang-tools-extra] r277336 - Changes related to new implementation of tooling::Replacements as class.

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 1 03:16:41 PDT 2016


Author: ioeric
Date: Mon Aug  1 05:16:39 2016
New Revision: 277336

URL: http://llvm.org/viewvc/llvm-project?rev=277336&view=rev
Log:
Changes related to new implementation of tooling::Replacements as class.

Summary: See http://reviews.llvm.org/D21748 for details.

Reviewers: djasper, klimek

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D21749

Modified:
    clang-tools-extra/trunk/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
    clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
    clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
    clang-tools-extra/trunk/clang-rename/RenamingAction.cpp
    clang-tools-extra/trunk/clang-rename/RenamingAction.h
    clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
    clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
    clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
    clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
    clang-tools-extra/trunk/tool-template/ToolTemplate.cpp
    clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Modified: clang-tools-extra/trunk/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h (original)
+++ clang-tools-extra/trunk/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h Mon Aug  1 05:16:39 2016
@@ -91,6 +91,11 @@ bool mergeAndDeduplicate(const TUReplace
                          FileToReplacementsMap &GroupedReplacements,
                          clang::SourceManager &SM);
 
+// FIXME: Remove this function after changing clang-apply-replacements to use
+// Replacements class.
+bool applyAllReplacements(const std::vector<tooling::Replacement> &Replaces,
+                          Rewriter &Rewrite);
+
 /// \brief Apply all replacements in \c GroupedReplacements.
 ///
 /// \param[in] GroupedReplacements Deduplicated and conflict free Replacements

Modified: clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp (original)
+++ clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp Mon Aug  1 05:16:39 2016
@@ -122,6 +122,81 @@ static void reportConflict(
   }
 }
 
+// FIXME: Remove this function after changing clang-apply-replacements to use
+// Replacements class.
+bool applyAllReplacements(const std::vector<tooling::Replacement> &Replaces,
+                          Rewriter &Rewrite) {
+  bool Result = true;
+  for (std::vector<tooling::Replacement>::const_iterator I = Replaces.begin(),
+                                                E = Replaces.end();
+       I != E; ++I) {
+    if (I->isApplicable()) {
+      Result = I->apply(Rewrite) && Result;
+    } else {
+      Result = false;
+    }
+  }
+  return Result;
+}
+
+
+// FIXME: moved from libToolingCore. remove this when std::vector<Replacement>
+// is replaced with tooling::Replacements class.
+static void deduplicate(std::vector<tooling::Replacement> &Replaces,
+                 std::vector<tooling::Range> &Conflicts) {
+  if (Replaces.empty())
+    return;
+
+  auto LessNoPath = [](const tooling::Replacement &LHS,
+                       const tooling::Replacement &RHS) {
+    if (LHS.getOffset() != RHS.getOffset())
+      return LHS.getOffset() < RHS.getOffset();
+    if (LHS.getLength() != RHS.getLength())
+      return LHS.getLength() < RHS.getLength();
+    return LHS.getReplacementText() < RHS.getReplacementText();
+  };
+
+  auto EqualNoPath = [](const tooling::Replacement &LHS,
+                        const tooling::Replacement &RHS) {
+    return LHS.getOffset() == RHS.getOffset() &&
+           LHS.getLength() == RHS.getLength() &&
+           LHS.getReplacementText() == RHS.getReplacementText();
+  };
+
+  // Deduplicate. We don't want to deduplicate based on the path as we assume
+  // that all replacements refer to the same file (or are symlinks).
+  std::sort(Replaces.begin(), Replaces.end(), LessNoPath);
+  Replaces.erase(std::unique(Replaces.begin(), Replaces.end(), EqualNoPath),
+                 Replaces.end());
+
+  // Detect conflicts
+  tooling::Range ConflictRange(Replaces.front().getOffset(),
+                               Replaces.front().getLength());
+  unsigned ConflictStart = 0;
+  unsigned ConflictLength = 1;
+  for (unsigned i = 1; i < Replaces.size(); ++i) {
+    tooling::Range Current(Replaces[i].getOffset(), Replaces[i].getLength());
+    if (ConflictRange.overlapsWith(Current)) {
+      // Extend conflicted range
+      ConflictRange =
+          tooling::Range(ConflictRange.getOffset(),
+                         std::max(ConflictRange.getLength(),
+                                  Current.getOffset() + Current.getLength() -
+                                      ConflictRange.getOffset()));
+      ++ConflictLength;
+    } else {
+      if (ConflictLength > 1)
+        Conflicts.push_back(tooling::Range(ConflictStart, ConflictLength));
+      ConflictRange = Current;
+      ConflictStart = i;
+      ConflictLength = 1;
+    }
+  }
+
+  if (ConflictLength > 1)
+    Conflicts.push_back(tooling::Range(ConflictStart, ConflictLength));
+}
+
 /// \brief Deduplicates and tests for conflicts among the replacements for each
 /// file in \c Replacements. Any conflicts found are reported.
 ///
@@ -144,7 +219,7 @@ static bool deduplicateAndDetectConflict
     assert(Entry != nullptr && "No file entry!");
 
     std::vector<tooling::Range> Conflicts;
-    tooling::deduplicate(FileAndReplacements.second, Conflicts);
+    deduplicate(FileAndReplacements.second, Conflicts);
 
     if (Conflicts.empty())
       continue;
@@ -197,7 +272,7 @@ bool applyReplacements(const FileToRepla
   // However, until we nail down the design of ReplacementGroups, might as well
   // leave this as is.
   for (const auto &FileAndReplacements : GroupedReplacements) {
-    if (!tooling::applyAllReplacements(FileAndReplacements.second, Rewrites))
+    if (!applyAllReplacements(FileAndReplacements.second, Rewrites))
       return false;
   }
 

Modified: clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp (original)
+++ clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp Mon Aug  1 05:16:39 2016
@@ -108,7 +108,7 @@ getRewrittenData(const std::vector<tooli
                  Rewriter &Rewrites, std::string &Result) {
   if (Replacements.empty()) return true;
 
-  if (!tooling::applyAllReplacements(Replacements, Rewrites))
+  if (!applyAllReplacements(Replacements, Rewrites))
     return false;
 
   SourceManager &SM = Rewrites.getSourceMgr();

Modified: clang-tools-extra/trunk/clang-rename/RenamingAction.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/RenamingAction.cpp?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-rename/RenamingAction.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/RenamingAction.cpp Mon Aug  1 05:16:39 2016
@@ -34,11 +34,13 @@ namespace rename {
 
 class RenamingASTConsumer : public ASTConsumer {
 public:
-  RenamingASTConsumer(const std::string &NewName, const std::string &PrevName,
-                      const std::vector<std::string> &USRs,
-                      tooling::Replacements &Replaces, bool PrintLocations)
-      : NewName(NewName), PrevName(PrevName), USRs(USRs), Replaces(Replaces),
-        PrintLocations(PrintLocations) {}
+  RenamingASTConsumer(
+      const std::string &NewName, const std::string &PrevName,
+      const std::vector<std::string> &USRs,
+      std::map<std::string, tooling::Replacements> &FileToReplaces,
+      bool PrintLocations)
+      : NewName(NewName), PrevName(PrevName), USRs(USRs),
+        FileToReplaces(FileToReplaces), PrintLocations(PrintLocations) {}
 
   void HandleTranslationUnit(ASTContext &Context) override {
     const auto &SourceMgr = Context.getSourceManager();
@@ -58,21 +60,25 @@ public:
                << ":" << FullLoc.getSpellingLineNumber() << ":"
                << FullLoc.getSpellingColumnNumber() << "\n";
       }
-      Replaces.insert(
-          tooling::Replacement(SourceMgr, Loc, PrevNameLen, NewName));
+      // FIXME: better error handling.
+      auto Replace = tooling::Replacement(SourceMgr, Loc, PrevNameLen, NewName);
+      auto Err = FileToReplaces[Replace.getFilePath()].add(Replace);
+      if (Err)
+        llvm::errs() << "Renaming failed in " << Replace.getFilePath() << "! "
+                     << llvm::toString(std::move(Err)) << "\n";
     }
   }
 
 private:
   const std::string &NewName, &PrevName;
   const std::vector<std::string> &USRs;
-  tooling::Replacements &Replaces;
+  std::map<std::string, tooling::Replacements> &FileToReplaces;
   bool PrintLocations;
 };
 
 std::unique_ptr<ASTConsumer> RenamingAction::newASTConsumer() {
   return llvm::make_unique<RenamingASTConsumer>(NewName, PrevName, USRs,
-                                                Replaces, PrintLocations);
+                                                FileToReplaces, PrintLocations);
 }
 
 } // namespace rename

Modified: clang-tools-extra/trunk/clang-rename/RenamingAction.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/RenamingAction.h?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-rename/RenamingAction.h (original)
+++ clang-tools-extra/trunk/clang-rename/RenamingAction.h Mon Aug  1 05:16:39 2016
@@ -27,17 +27,17 @@ class RenamingAction {
 public:
   RenamingAction(const std::string &NewName, const std::string &PrevName,
                  const std::vector<std::string> &USRs,
-                 tooling::Replacements &Replaces, bool PrintLocations = false)
-      : NewName(NewName), PrevName(PrevName), USRs(USRs), Replaces(Replaces),
-        PrintLocations(PrintLocations) {
-  }
+                 std::map<std::string, tooling::Replacements> &FileToReplaces,
+                 bool PrintLocations = false)
+      : NewName(NewName), PrevName(PrevName), USRs(USRs),
+        FileToReplaces(FileToReplaces), PrintLocations(PrintLocations) {}
 
   std::unique_ptr<ASTConsumer> newASTConsumer();
 
 private:
   const std::string &NewName, &PrevName;
   const std::vector<std::string> &USRs;
-  tooling::Replacements &Replaces;
+  std::map<std::string, tooling::Replacements> &FileToReplaces;
   bool PrintLocations;
 };
 

Modified: clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp Mon Aug  1 05:16:39 2016
@@ -146,9 +146,10 @@ int main(int argc, const char **argv) {
 
       // Export replacements.
       tooling::TranslationUnitReplacements TUR;
-      const tooling::Replacements &Replacements = Tool.getReplacements();
-      TUR.Replacements.insert(TUR.Replacements.end(), Replacements.begin(),
-                              Replacements.end());
+      const auto &FileToReplacements = Tool.getReplacements();
+      for (const auto &Entry : FileToReplacements)
+        TUR.Replacements.insert(TUR.Replacements.end(), Entry.second.begin(),
+                                Entry.second.end());
 
       yaml::Output YAML(OS);
       YAML << TUR;

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Mon Aug  1 05:16:39 2016
@@ -77,7 +77,14 @@ protected:
       assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() &&
              "Only file locations supported in fix-it hints.");
 
-      Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert));
+      auto Err =
+          Error.Fix.add(tooling::Replacement(SM, Range, FixIt.CodeToInsert));
+      // FIXME: better error handling.
+      if (Err) {
+        llvm::errs() << "Fix conflicts with existing fix! "
+                    << llvm::toString(std::move(Err)) << "\n";
+      }
+      assert(!Err && "Fix conflicts with existing fix!");
     }
   }
 

Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Mon Aug  1 05:16:39 2016
@@ -352,23 +352,36 @@ llvm::Expected<tooling::Replacements> cr
   std::string IncludeName =
       "#include " + Context.getHeaderInfos().front().Header + "\n";
   // Create replacements for the new header.
-  clang::tooling::Replacements Insertions = {
-      tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName)};
+  clang::tooling::Replacements Insertions;
+  auto Err =
+      Insertions.add(tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName));
+  if (Err)
+    return std::move(Err);
 
   auto CleanReplaces = cleanupAroundReplacements(Code, Insertions, Style);
   if (!CleanReplaces)
     return CleanReplaces;
 
+  auto Replaces = std::move(*CleanReplaces);
   if (AddQualifiers) {
     for (const auto &Info : Context.getQuerySymbolInfos()) {
       // Ignore the empty range.
-      if (Info.Range.getLength() > 0)
-        CleanReplaces->insert({FilePath, Info.Range.getOffset(),
-                               Info.Range.getLength(),
-                               Context.getHeaderInfos().front().QualifiedName});
+      if (Info.Range.getLength() > 0) {
+        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));
+        }
+      }
     }
   }
-  return formatReplacements(Code, *CleanReplaces, Style);
+  return formatReplacements(Code, Replaces, Style);
 }
 
 } // namespace include_fixer

Modified: clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp Mon Aug  1 05:16:39 2016
@@ -350,8 +350,8 @@ int includeFixerMain(int argc, const cha
   }
 
   if (!Quiet)
-    errs() << "Added #include " << Context.getHeaderInfos().front().Header
-           << '\n';
+    llvm::errs() << "Added #include " << Context.getHeaderInfos().front().Header
+                 << "\n";
 
   // Set up a new source manager for applying the resulting replacements.
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions);

Modified: clang-tools-extra/trunk/tool-template/ToolTemplate.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/tool-template/ToolTemplate.cpp?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/tool-template/ToolTemplate.cpp (original)
+++ clang-tools-extra/trunk/tool-template/ToolTemplate.cpp Mon Aug  1 05:16:39 2016
@@ -53,18 +53,20 @@ using namespace llvm;
 
 namespace {
 class ToolTemplateCallback : public MatchFinder::MatchCallback {
- public:
-  ToolTemplateCallback(Replacements *Replace) : Replace(Replace) {}
+public:
+  ToolTemplateCallback(std::map<std::string, Replacements> *Replace)
+      : Replace(Replace) {}
 
   void run(const MatchFinder::MatchResult &Result) override {
-    // TODO: This routine will get called for each thing that the matchers find.
+    // TODO: This routine will get called for each thing that the matchers
+    // find.
     // At this point, you can examine the match, and do whatever you want,
     // including replacing the matched text with other text
     (void)Replace; // This to prevent an "unused member variable" warning;
   }
 
- private:
-  Replacements *Replace;
+private:
+  std::map<std::string, Replacements> *Replace;
 };
 } // end anonymous namespace
 

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=277336&r1=277335&r2=277336&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Mon Aug  1 05:16:39 2016
@@ -119,7 +119,14 @@ runCheckOnCode(StringRef Code, std::vect
   DiagConsumer.finish();
   tooling::Replacements Fixes;
   for (const ClangTidyError &Error : Context.getErrors())
-    Fixes.insert(Error.Fix.begin(), Error.Fix.end());
+    for (const auto &Fix : Error.Fix) {
+      auto Err = Fixes.add(Fix);
+      // FIXME: better error handling. Keep the behavior for now.
+      if (Err) {
+        llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+        return "";
+      }
+    }
   if (Errors)
     *Errors = Context.getErrors();
   auto Result = tooling::applyAllReplacements(Code, Fixes);




More information about the cfe-commits mailing list