clang-modernize: Reset LoopConvert's TU tracking info per TU

The LoopConvert transform makes use of data structures it builds up over
the course of transforming a TU. Until now, these data structures
weren't being cleared out before the next TU was being processed.

Fixes PR17253.

Differential Revision: http://llvm-reviews.chandlerc.com/D175

llvm-svn: 191448

clang-tools-extra/clang-modernize/LoopConvert/LoopActions.cpp

@@ -821,7 +821,8 @@ void LoopFixer::doConversion(ASTContext *Context,
     // No further replacements are made to the loop, since the iterator or index
     // was used exactly once - in the initialization of AliasVar.
   } else {
-    VariableNamer Namer(GeneratedDecls, &ParentFinder->getStmtToParentStmtMap(),
+    VariableNamer Namer(&TUInfo.getGeneratedDecls(),
+                        &TUInfo.getParentFinder().getStmtToParentStmtMap(),
                         TheLoop, IndexVar, MaybeContainer, Context);
     VarName = Namer.createIndexName();
     // First, replace all usages of the array subscript expression with our new
@@ -829,7 +830,7 @@ void LoopFixer::doConversion(ASTContext *Context,
     for (UsageResult::const_iterator I = Usages.begin(), E = Usages.end();
          I != E; ++I) {
       std::string ReplaceText = I->IsArrow ? VarName + "." : VarName;
-      ReplacedVarRanges->insert(std::make_pair(TheLoop, IndexVar));
+      TUInfo.getReplacedVars().insert(std::make_pair(TheLoop, IndexVar));
       Owner.addReplacementForCurrentTU(
           Replacement(Context->getSourceManager(),
                       CharSourceRange::getTokenRange(I->Range), ReplaceText));
@@ -864,7 +865,7 @@ void LoopFixer::doConversion(ASTContext *Context,
   Owner.addReplacementForCurrentTU(
       Replacement(Context->getSourceManager(),
                   CharSourceRange::getTokenRange(ParenRange), Range));
-  GeneratedDecls->insert(make_pair(TheLoop, VarName));
+  TUInfo.getGeneratedDecls().insert(make_pair(TheLoop, VarName));
 }
 
 /// \brief Determine whether Init appears to be an initializing an iterator.
@@ -933,18 +934,18 @@ StringRef LoopFixer::checkDeferralsAndRejections(ASTContext *Context,
   // updates on this iteration.
   // FIXME: Once Replacements can detect conflicting edits, replace this
   // implementation and rely on conflicting edit detection instead.
-  if (ReplacedVarRanges->count(TheLoop)) {
+  if (TUInfo.getReplacedVars().count(TheLoop)) {
     ++*DeferredChanges;
     return "";
   }
 
-  ParentFinder->gatherAncestors(Context->getTranslationUnitDecl());
+  TUInfo.getParentFinder().gatherAncestors(Context->getTranslationUnitDecl());
   // Ensure that we do not try to move an expression dependent on a local
   // variable declared inside the loop outside of it!
-  DependencyFinderASTVisitor
+  DependencyFinderASTVisitor DependencyFinder(
-      DependencyFinder(&ParentFinder->getStmtToParentStmtMap(),
+      &TUInfo.getParentFinder().getStmtToParentStmtMap(),
-                       &ParentFinder->getDeclToParentStmtMap(),
+      &TUInfo.getParentFinder().getDeclToParentStmtMap(),
-                       ReplacedVarRanges, TheLoop);
+      &TUInfo.getReplacedVars(), TheLoop);
 
   // Not all of these are actually deferred changes.
   // FIXME: Determine when the external dependency isn't an expression converted

clang-tools-extra/clang-modernize/LoopConvert/LoopActions.h

@@ -34,30 +34,47 @@ enum LoopFixerKind {
   LFK_PseudoArray
 };
 
+struct TUTrackingInfo {
+
+  /// \brief Reset and initialize per-TU tracking information.
+  ///
+  /// Must be called before using container accessors.
+  void reset() {
+    ParentFinder.reset(new StmtAncestorASTVisitor);
+    GeneratedDecls.clear();
+    ReplacedVars.clear();
+  }
+
+  /// \name Accessors
+  /// \{
+  StmtAncestorASTVisitor &getParentFinder() { return *ParentFinder; }
+  StmtGeneratedVarNameMap &getGeneratedDecls() { return GeneratedDecls; }
+  ReplacedVarsMap &getReplacedVars() { return ReplacedVars; }
+  /// \}
+
+private:
+  llvm::OwningPtr<StmtAncestorASTVisitor> ParentFinder;
+  StmtGeneratedVarNameMap GeneratedDecls;
+  ReplacedVarsMap ReplacedVars;
+};
+
 /// \brief The callback to be used for loop migration matchers.
 ///
 /// The callback does extra checking not possible in matchers, and attempts to
 /// convert the for loop, if possible.
 class LoopFixer : public clang::ast_matchers::MatchFinder::MatchCallback {
- public:
+public:
-   LoopFixer(StmtAncestorASTVisitor *ParentFinder,
+  LoopFixer(TUTrackingInfo &TUInfo, unsigned *AcceptedChanges,
-             StmtGeneratedVarNameMap *GeneratedDecls,
+            unsigned *DeferredChanges, unsigned *RejectedChanges,
-             ReplacedVarsMap *ReplacedVarRanges, unsigned *AcceptedChanges,
+            RiskLevel MaxRisk, LoopFixerKind FixerKind, Transform &Owner)
-             unsigned *DeferredChanges, unsigned *RejectedChanges,
+      : TUInfo(TUInfo), AcceptedChanges(AcceptedChanges),
-             RiskLevel MaxRisk, LoopFixerKind FixerKind, Transform &Owner)
+        DeferredChanges(DeferredChanges), RejectedChanges(RejectedChanges),
-       : ParentFinder(ParentFinder),
+        MaxRisk(MaxRisk), FixerKind(FixerKind), Owner(Owner) {}
-         GeneratedDecls(GeneratedDecls), ReplacedVarRanges(ReplacedVarRanges),
-         AcceptedChanges(AcceptedChanges), DeferredChanges(DeferredChanges),
-         RejectedChanges(RejectedChanges), MaxRisk(MaxRisk),
-         FixerKind(FixerKind), Owner(Owner) {}
 
-   virtual void
+  virtual void run(const clang::ast_matchers::MatchFinder::MatchResult &Result);
-       run(const clang::ast_matchers::MatchFinder::MatchResult &Result);
 
- private:
+private:
-  StmtAncestorASTVisitor *ParentFinder;
+  TUTrackingInfo &TUInfo;
-  StmtGeneratedVarNameMap *GeneratedDecls;
-  ReplacedVarsMap *ReplacedVarRanges;
   unsigned *AcceptedChanges;
   unsigned *DeferredChanges;
   unsigned *RejectedChanges;
@@ -67,17 +84,12 @@ class LoopFixer : public clang::ast_matchers::MatchFinder::MatchCallback {
 
   /// \brief Computes the changes needed to convert a given for loop, and
   /// applies it.
-  void doConversion(clang::ASTContext *Context,
+  void doConversion(clang::ASTContext *Context, const clang::VarDecl *IndexVar,
-                    const clang::VarDecl *IndexVar,
                     const clang::VarDecl *MaybeContainer,
-                    llvm::StringRef ContainerString,
+                    llvm::StringRef ContainerString, const UsageResult &Usages,
-                    const UsageResult &Usages,
+                    const clang::DeclStmt *AliasDecl, bool AliasUseRequired,
-                    const clang::DeclStmt *AliasDecl,
+                    bool AliasFromForInit, const clang::ForStmt *TheLoop,
-                    bool AliasUseRequired,
+                    bool ContainerNeedsDereference, bool DerefByValue,
-                    bool AliasFromForInit,
-                    const clang::ForStmt *TheLoop,
-                    bool ContainerNeedsDereference,
-                    bool DerefByValue,
                     bool DerefByConstRef);
 
   /// \brief Given a loop header that would be convertible, discover all usages
@@ -87,10 +99,8 @@ class LoopFixer : public clang::ast_matchers::MatchFinder::MatchCallback {
                            const clang::VarDecl *EndVar,
                            const clang::Expr *ContainerExpr,
                            const clang::Expr *BoundExpr,
-                           bool ContainerNeedsDereference,
+                           bool ContainerNeedsDereference, bool DerefByValue,
-                           bool DerefByValue,
+                           bool DerefByConstRef, const clang::ForStmt *TheLoop,
-                           bool DerefByConstRef,
-                           const clang::ForStmt *TheLoop,
                            Confidence ConfidenceLevel);
 
   /// \brief Determine if the change should be deferred or rejected, returning

clang-tools-extra/clang-modernize/LoopConvert/LoopConvert.cpp

@@ -29,26 +29,22 @@ int LoopConvertTransform::apply(const FileOverrides &InputStates,
                                 const std::vector<std::string> &SourcePaths) {
   ClangTool LoopTool(Database, SourcePaths);
 
-  StmtAncestorASTVisitor ParentFinder;
-  StmtGeneratedVarNameMap GeneratedDecls;
-  ReplacedVarsMap ReplacedVars;
   unsigned AcceptedChanges = 0;
   unsigned DeferredChanges = 0;
   unsigned RejectedChanges = 0;
 
+  TUInfo.reset(new TUTrackingInfo);
+
   MatchFinder Finder;
-  LoopFixer ArrayLoopFixer(&ParentFinder, &GeneratedDecls, &ReplacedVars,
+  LoopFixer ArrayLoopFixer(*TUInfo, &AcceptedChanges, &DeferredChanges,
-                           &AcceptedChanges, &DeferredChanges, &RejectedChanges,
+                           &RejectedChanges, Options().MaxRiskLevel, LFK_Array,
-                           Options().MaxRiskLevel, LFK_Array,
                            /*Owner=*/ *this);
   Finder.addMatcher(makeArrayLoopMatcher(), &ArrayLoopFixer);
-  LoopFixer IteratorLoopFixer(&ParentFinder, &GeneratedDecls, &ReplacedVars,
+  LoopFixer IteratorLoopFixer(*TUInfo, &AcceptedChanges, &DeferredChanges,
-                              &AcceptedChanges, &DeferredChanges,
                               &RejectedChanges, Options().MaxRiskLevel,
                               LFK_Iterator, /*Owner=*/ *this);
   Finder.addMatcher(makeIteratorLoopMatcher(), &IteratorLoopFixer);
-  LoopFixer PseudoarrrayLoopFixer(&ParentFinder, &GeneratedDecls, &ReplacedVars,
+  LoopFixer PseudoarrrayLoopFixer(*TUInfo, &AcceptedChanges, &DeferredChanges,
-                                  &AcceptedChanges, &DeferredChanges,
                                   &RejectedChanges, Options().MaxRiskLevel,
                                   LFK_PseudoArray, /*Owner=*/ *this);
   Finder.addMatcher(makePseudoArrayLoopMatcher(), &PseudoarrrayLoopFixer);
@@ -67,6 +63,15 @@ int LoopConvertTransform::apply(const FileOverrides &InputStates,
   return 0;
 }
 
+bool
+LoopConvertTransform::handleBeginSource(clang::CompilerInstance &CI,
+                                        llvm::StringRef Filename) {
+  // Reset and initialize per-TU tracking structures.
+  TUInfo->reset();
+
+  return Transform::handleBeginSource(CI, Filename);
+}
+
 struct LoopConvertFactory : TransformFactory {
   LoopConvertFactory() {
     Since.Clang = Version(3, 0);

clang-tools-extra/clang-modernize/LoopConvert/LoopConvert.h

@@ -20,6 +20,9 @@
 #include "Core/Transform.h"
 #include "llvm/Support/Compiler.h" // For LLVM_OVERRIDE
 
+// Forward decl for private implementation.
+struct TUTrackingInfo;
+
 /// \brief Subclass of Transform that transforms for-loops into range-based
 /// for-loops where possible.
 class LoopConvertTransform : public Transform {
@@ -31,6 +34,11 @@ public:
   virtual int apply(const FileOverrides &InputStates,
                     const clang::tooling::CompilationDatabase &Database,
                     const std::vector<std::string> &SourcePaths) LLVM_OVERRIDE;
+
+  virtual bool handleBeginSource(clang::CompilerInstance &CI,
+                                 llvm::StringRef Filename) LLVM_OVERRIDE;
+private:
+  llvm::OwningPtr<TUTrackingInfo> TUInfo;
 };
 
 #endif // CLANG_MODERNIZE_LOOP_CONVERT_H