commit aec230d29835285777ecc467e268c83b33a2adde
author Argyrios Kyrtzidis <akyrtzi@gmail.com> Thu Sep 01 20:53:18 2011 +0000
committer Argyrios Kyrtzidis <akyrtzi@gmail.com> Thu Sep 01 20:53:18 2011 +0000
tree 71d407d2a7ce65d1cbf4288c0838fdb12e68ad98
parent 3e3cd93b2fd9644e970c389e715c13883faf68b6

[arcmt] Fix test/ARCMT/remove-statements.m regression due to
Objective-C method buffering(rdar://10056942)

Turned out the same issue existed for C++ inline methods.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@138960 91177308-0d34-0410-b5e6-96231b3b80d8

lib/ARCMigrate/TransEmptyStatementsAndDealloc.cpp

diff --git a/lib/ARCMigrate/TransEmptyStatementsAndDealloc.cpp b/lib/ARCMigrate/TransEmptyStatementsAndDealloc.cpp
index b896c48..0b3f4c3 100644
--- a/lib/ARCMigrate/TransEmptyStatementsAndDealloc.cpp
+++ b/lib/ARCMigrate/TransEmptyStatementsAndDealloc.cpp
@@ -22,25 +22,68 @@
 #include "Transforms.h"
 #include "Internals.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/Basic/SourceManager.h"
 
 using namespace clang;
 using namespace arcmt;
 using namespace trans;
 
+static bool isEmptyARCMTMacroStatement(NullStmt *S,
+                                       std::vector<SourceLocation> &MacroLocs,
+                                       ASTContext &Ctx) {
+  if (!S->hasLeadingEmptyMacro())
+    return false;
+
+  SourceLocation SemiLoc = S->getSemiLoc();
+  if (SemiLoc.isInvalid() || SemiLoc.isMacroID())
+    return false;
+
+  if (MacroLocs.empty())
+    return false;
+
+  SourceManager &SM = Ctx.getSourceManager();
+  std::vector<SourceLocation>::iterator
+    I = std::upper_bound(MacroLocs.begin(), MacroLocs.end(), SemiLoc,
+                         SourceManager::LocBeforeThanCompare(SM));
+  --I;
+  SourceLocation
+      AfterMacroLoc = I->getFileLocWithOffset(getARCMTMacroName().size());
+  assert(AfterMacroLoc.isFileID());
+
+  if (AfterMacroLoc == SemiLoc)
+    return true;
+
+  int RelOffs = 0;
+  if (!SM.isInSameSLocAddrSpace(AfterMacroLoc, SemiLoc, &RelOffs))
+    return false;
+  if (RelOffs < 0)
+    return false;
+
+  // We make the reasonable assumption that a semicolon after 100 characters
+  // means that it is not the next token after our macro. If this assumption
+  // fails it is not critical, we will just fail to clear out, e.g., an empty
+  // 'if'.
+  if (RelOffs - getARCMTMacroName().size() > 100)
+    return false;
+
+  SourceLocation AfterMacroSemiLoc = findSemiAfterLocation(AfterMacroLoc, Ctx);
+  return AfterMacroSemiLoc == SemiLoc;
+}
+
 namespace {
 
 /// \brief Returns true if the statement became empty due to previous
 /// transformations.
 class EmptyChecker : public StmtVisitor<EmptyChecker, bool> {
   ASTContext &Ctx;
-  llvm::DenseSet<unsigned> &MacroLocs;
+  std::vector<SourceLocation> &MacroLocs;
 
 public:
-  EmptyChecker(ASTContext &ctx, llvm::DenseSet<unsigned> &macroLocs)
+  EmptyChecker(ASTContext &ctx, std::vector<SourceLocation> &macroLocs)
     : Ctx(ctx), MacroLocs(macroLocs) { }
 
   bool VisitNullStmt(NullStmt *S) {
-    return isMacroLoc(S->getLeadingEmptyMacroLoc());
+    return isEmptyARCMTMacroStatement(S, MacroLocs, Ctx);
   }
   bool VisitCompoundStmt(CompoundStmt *S) {
     if (S->body_empty())
@@ -102,23 +145,14 @@
       return false;
     return Visit(S->getSubStmt());
   }
-
-private:
-  bool isMacroLoc(SourceLocation loc) {
-    if (loc.isInvalid()) return false;
-    return MacroLocs.count(loc.getRawEncoding());
-  }
 };
 
 class EmptyStatementsRemover :
                             public RecursiveASTVisitor<EmptyStatementsRemover> {
   MigrationPass &Pass;
-  llvm::DenseSet<unsigned> &MacroLocs;
 
 public:
-  EmptyStatementsRemover(MigrationPass &pass,
-                         llvm::DenseSet<unsigned> &macroLocs)
-    : Pass(pass), MacroLocs(macroLocs) { }
+  EmptyStatementsRemover(MigrationPass &pass) : Pass(pass) { }
 
   bool TraverseStmtExpr(StmtExpr *E) {
     CompoundStmt *S = E->getSubStmt();
@@ -138,17 +172,12 @@
     return true;
   }
 
-  bool isMacroLoc(SourceLocation loc) {
-    if (loc.isInvalid()) return false;
-    return MacroLocs.count(loc.getRawEncoding());
-  }
-
   ASTContext &getContext() { return Pass.Ctx; }
 
 private:
   void check(Stmt *S) {
     if (!S) return;
-    if (EmptyChecker(Pass.Ctx, MacroLocs).Visit(S)) {
+    if (EmptyChecker(Pass.Ctx, Pass.ARCMTMacroLocs).Visit(S)) {
       Transaction Trans(Pass.TA);
       Pass.TA.removeStmt(S);
     }
@@ -157,8 +186,8 @@
 
 } // anonymous namespace
 
-static bool isBodyEmpty(CompoundStmt *body,
-                        ASTContext &Ctx, llvm::DenseSet<unsigned> &MacroLocs) {
+static bool isBodyEmpty(CompoundStmt *body, ASTContext &Ctx,
+                        std::vector<SourceLocation> &MacroLocs) {
   for (CompoundStmt::body_iterator
          I = body->body_begin(), E = body->body_end(); I != E; ++I)
     if (!EmptyChecker(Ctx, MacroLocs).Visit(*I))
@@ -167,8 +196,7 @@
   return true;
 }
 
-static void removeDeallocMethod(MigrationPass &pass,
-                                llvm::DenseSet<unsigned> &MacroLocs) {
+static void removeDeallocMethod(MigrationPass &pass) {
   ASTContext &Ctx = pass.Ctx;
   TransformActions &TA = pass.TA;
   DeclContext *DC = Ctx.getTranslationUnitDecl();
@@ -183,7 +211,7 @@
       ObjCMethodDecl *MD = *MI;
       if (MD->getMethodFamily() == OMF_dealloc) {
         if (MD->hasBody() &&
-            isBodyEmpty(MD->getCompoundBody(), Ctx, MacroLocs)) {
+            isBodyEmpty(MD->getCompoundBody(), Ctx, pass.ARCMTMacroLocs)) {
           Transaction Trans(TA);
           TA.remove(MD->getSourceRange());
         }
@@ -194,14 +222,9 @@
 }
 
 void trans::removeEmptyStatementsAndDealloc(MigrationPass &pass) {
-  llvm::DenseSet<unsigned> MacroLocs;
-  for (unsigned i = 0, e = pass.ARCMTMacroLocs.size(); i != e; ++i)
-    MacroLocs.insert(pass.ARCMTMacroLocs[i].getRawEncoding());
+  EmptyStatementsRemover(pass).TraverseDecl(pass.Ctx.getTranslationUnitDecl());
 
-  EmptyStatementsRemover(pass, MacroLocs)
-    .TraverseDecl(pass.Ctx.getTranslationUnitDecl());
-
-  removeDeallocMethod(pass, MacroLocs);
+  removeDeallocMethod(pass);
 
   for (unsigned i = 0, e = pass.ARCMTMacroLocs.size(); i != e; ++i) {
     Transaction Trans(pass.TA);