[arcmt] Adds brackets in case statements that "contain" initialization of retaining

variable, thus emitting the "switch case is in protected scope" error.

rdar://12952016

llvm-svn: 171484

clang/lib/ARCMigrate/ARCMT.cpp

@@ -39,8 +39,9 @@ bool CapturedDiagList::clearDiagnostic(ArrayRef<unsigned> IDs,
            diagLoc.isBeforeInTranslationUnitThan(range.getEnd()))) {
       cleared = true;
       ListTy::iterator eraseS = I++;
-      while (I != List.end() && I->getLevel() == DiagnosticsEngine::Note)
-        ++I;
+      if (eraseS->getLevel() != DiagnosticsEngine::Note)
+        while (I != List.end() && I->getLevel() == DiagnosticsEngine::Note)
+          ++I;
       // Clear the diagnostic and any notes following it.
       I = List.erase(eraseS, I);
       continue;
@@ -296,7 +297,8 @@ bool arcmt::checkForManualIssues(CompilerInvocation &origCI,
   std::vector<SourceLocation> ARCMTMacroLocs;
 
   TransformActions testAct(*Diags, capturedDiags, Ctx, Unit->getPreprocessor());
-  MigrationPass pass(Ctx, OrigGCMode, Unit->getSema(), testAct, ARCMTMacroLocs);
+  MigrationPass pass(Ctx, OrigGCMode, Unit->getSema(), testAct, capturedDiags,
+                     ARCMTMacroLocs);
   pass.setNSAllocReallocError(NoNSAllocReallocError);
   pass.setNoFinalizeRemoval(NoFinalizeRemoval);
 
@@ -599,7 +601,7 @@ bool MigrationProcess::applyTransform(TransformFn trans,
   Rewriter rewriter(Ctx.getSourceManager(), Ctx.getLangOpts());
   TransformActions TA(*Diags, capturedDiags, Ctx, Unit->getPreprocessor());
   MigrationPass pass(Ctx, OrigCI.getLangOpts()->getGC(),
-                     Unit->getSema(), TA, ARCMTMacroLocs);
+                     Unit->getSema(), TA, capturedDiags, ARCMTMacroLocs);
 
   trans(pass);
 

clang/lib/ARCMigrate/Internals.h

@@ -146,16 +146,20 @@ public:
   MigratorOptions MigOptions;
   Sema &SemaRef;
   TransformActions &TA;
+  const CapturedDiagList &CapturedDiags;
   std::vector<SourceLocation> &ARCMTMacroLocs;
   llvm::Optional<bool> EnableCFBridgeFns;
 
   MigrationPass(ASTContext &Ctx, LangOptions::GCMode OrigGCMode,
                 Sema &sema, TransformActions &TA,
+                const CapturedDiagList &capturedDiags,
                 std::vector<SourceLocation> &ARCMTMacroLocs)
     : Ctx(Ctx), OrigGCMode(OrigGCMode), MigOptions(),
-      SemaRef(sema), TA(TA),
+      SemaRef(sema), TA(TA), CapturedDiags(capturedDiags),
       ARCMTMacroLocs(ARCMTMacroLocs) { }
 
+  const CapturedDiagList &getDiags() const { return CapturedDiags; }
+
   bool isGCMigration() const { return OrigGCMode != LangOptions::NonGC; }
   bool noNSAllocReallocError() const { return MigOptions.NoNSAllocReallocError; }
   void setNSAllocReallocError(bool val) { MigOptions.NoNSAllocReallocError = val; }

clang/lib/ARCMigrate/TransProtectedScope.cpp

@@ -0,0 +1,118 @@
+//===--- TransProtectedScope.cpp - Transformations to ARC mode ------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Adds brackets in case statements that "contain" initialization of retaining
+// variable, thus emitting the "switch case is in protected scope" error.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Transforms.h"
+#include "Internals.h"
+#include "clang/Sema/SemaDiagnostic.h"
+
+using namespace clang;
+using namespace arcmt;
+using namespace trans;
+
+namespace {
+
+struct CaseInfo {
+  SwitchCase *SC;
+  SourceRange Range;
+  bool FixedBypass;
+  
+  CaseInfo() : SC(0), FixedBypass(false) {}
+  CaseInfo(SwitchCase *S, SourceRange Range)
+    : SC(S), Range(Range), FixedBypass(false) {}
+};
+
+class CaseCollector : public RecursiveASTVisitor<CaseCollector> {
+  llvm::SmallVectorImpl<CaseInfo> &Cases;
+
+public:
+  CaseCollector(llvm::SmallVectorImpl<CaseInfo> &Cases)
+    : Cases(Cases) { }
+
+  bool VisitSwitchStmt(SwitchStmt *S) {
+    SourceLocation NextLoc = S->getLocEnd();
+    SwitchCase *Curr = S->getSwitchCaseList();
+    // We iterate over case statements in reverse source-order.
+    while (Curr) {
+      Cases.push_back(CaseInfo(Curr,SourceRange(Curr->getLocStart(), NextLoc)));
+      NextLoc = Curr->getLocStart();
+      Curr = Curr->getNextSwitchCase();
+    }
+    return true;
+  }
+};
+
+} // anonymous namespace
+
+static bool isInRange(FullSourceLoc Loc, SourceRange R) {
+  return !Loc.isBeforeInTranslationUnitThan(R.getBegin()) &&
+          Loc.isBeforeInTranslationUnitThan(R.getEnd());
+}
+
+static bool handleProtectedNote(const StoredDiagnostic &Diag,
+                                llvm::SmallVectorImpl<CaseInfo> &Cases,
+                                TransformActions &TA) {
+  assert(Diag.getLevel() == DiagnosticsEngine::Note);
+
+  for (unsigned i = 0; i != Cases.size(); i++) {
+    CaseInfo &info = Cases[i];
+    if (isInRange(Diag.getLocation(), info.Range)) {
+      TA.clearDiagnostic(Diag.getID(), Diag.getLocation());
+      if (!info.FixedBypass) {
+        TA.insertAfterToken(info.SC->getColonLoc(), " {");
+        TA.insert(info.Range.getEnd(), "}\n");
+        info.FixedBypass = true;
+      }
+      return true;
+    }
+  }
+
+  return false;
+}
+
+static void handleProtectedScopeError(CapturedDiagList::iterator &DiagI,
+                                      CapturedDiagList::iterator DiagE,
+                                      llvm::SmallVectorImpl<CaseInfo> &Cases,
+                                      TransformActions &TA) {
+  Transaction Trans(TA);
+  assert(DiagI->getID() == diag::err_switch_into_protected_scope);
+  SourceLocation ErrLoc = DiagI->getLocation();
+  bool handledAllNotes = true;
+  ++DiagI;
+  for (; DiagI != DiagE && DiagI->getLevel() == DiagnosticsEngine::Note;
+       ++DiagI) {
+    if (!handleProtectedNote(*DiagI, Cases, TA))
+      handledAllNotes = false;
+  }
+
+  if (handledAllNotes)
+    TA.clearDiagnostic(diag::err_switch_into_protected_scope, ErrLoc);
+}
+
+void ProtectedScopeTraverser::traverseBody(BodyContext &BodyCtx) {
+  MigrationPass &Pass = BodyCtx.getMigrationContext().Pass;
+  SmallVector<CaseInfo, 16> Cases;
+  CaseCollector(Cases).TraverseStmt(BodyCtx.getTopStmt());
+
+  SourceRange BodyRange = BodyCtx.getTopStmt()->getSourceRange();
+  const CapturedDiagList &DiagList = Pass.getDiags();
+  CapturedDiagList::iterator I = DiagList.begin(), E = DiagList.end();
+  while (I != E) {
+    if (I->getID() == diag::err_switch_into_protected_scope &&
+        isInRange(I->getLocation(), BodyRange)) {
+      handleProtectedScopeError(I, E, Cases, Pass.TA);
+      continue;
+    }
+    ++I;
+  }
+}

clang/lib/ARCMigrate/Transforms.cpp

@@ -564,6 +564,7 @@ static void traverseAST(MigrationPass &pass) {
   }
   MigrateCtx.addTraverser(new PropertyRewriteTraverser());
   MigrateCtx.addTraverser(new BlockObjCVariableTraverser());
+  MigrateCtx.addTraverser(new ProtectedScopeTraverser());
 
   MigrateCtx.traverse(pass.Ctx.getTranslationUnitDecl());
 }

clang/lib/ARCMigrate/Transforms.h

@@ -135,6 +135,11 @@ public:
   virtual void traverseBody(BodyContext &BodyCtx);
 };
 
+class ProtectedScopeTraverser : public ASTTraverser {
+public:
+  virtual void traverseBody(BodyContext &BodyCtx);
+};
+
 // GC transformations
 
 class GCAttrsTraverser : public ASTTraverser {

clang/test/ARCMT/checking.m

@@ -178,13 +178,12 @@ void test12(id collection) {
 }
 
 void test6(unsigned cond) {
-  // FIXME: Fix this automatically ?
   switch (cond) {
   case 0:
     ;
-    id x; // expected-note {{jump bypasses initialization of retaining variable}}
+    id x;
 
-  case 1: // expected-error {{switch case is in protected scope}}
+  case 1:
     break;
   }
 }
@@ -293,10 +292,10 @@ id test9(Test9 *v) {
 void rdar9491791(int p) {
   switch (p) {
   case 3:;
-    NSObject *o = [[NSObject alloc] init]; // expected-note {{jump bypasses initialization of retaining variable}}
+    NSObject *o = [[NSObject alloc] init];
     [o release];
     break;
-  default: // expected-error {{switch case is in protected scope}}
+  default:
     break;
   }
 }

clang/test/ARCMT/protected-scope.m

@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -fobjc-arc -x objective-c %s.result
+// RUN: arcmt-test --args -triple x86_64-apple-darwin10 -fsyntax-only -x objective-c %s > %t
+// RUN: diff %t %s.result
+// DISABLE: mingw32
+
+#include "Common.h"
+
+void test(id p, int x) {
+  int v;
+  switch(x) {
+  case 0:
+    v++;
+    id w1 = p;
+    id w2 = p;
+    break;
+  case 1:
+    v++;
+    id w3 = p;
+    break;
+  case 2:
+    break;
+  default:
+    break;
+  }
+}
+
+void test2(int p) {
+  switch (p) {
+  case 3:;
+    NSObject *o = [[NSObject alloc] init];
+    [o release];
+    break;
+  default:
+    break;
+  }
+}

clang/test/ARCMT/protected-scope.m.result

@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -fobjc-arc -x objective-c %s.result
+// RUN: arcmt-test --args -triple x86_64-apple-darwin10 -fsyntax-only -x objective-c %s > %t
+// RUN: diff %t %s.result
+// DISABLE: mingw32
+
+#include "Common.h"
+
+void test(id p, int x) {
+  int v;
+  switch(x) {
+  case 0: {
+    v++;
+    id w1 = p;
+    id w2 = p;
+    break;
+  }
+  case 1: {
+    v++;
+    id w3 = p;
+    break;
+  }
+  case 2:
+    break;
+  default:
+    break;
+  }
+}
+
+void test2(int p) {
+  switch (p) {
+  case 3: {;
+    NSObject *o = [[NSObject alloc] init];
+    break;
+  }
+  default:
+    break;
+  }
+}