[cfe-commits] r171484 - in /cfe/trunk: lib/ARCMigrate/ARCMT.cpp lib/ARCMigrate/Internals.h lib/ARCMigrate/TransProtectedScope.cpp lib/ARCMigrate/Transforms.cpp lib/ARCMigrate/Transforms.h test/ARCMT/checking.m test/ARCMT/protected-scope.m test/ARCMT/protected-scope.m.result

Jordan Rose jordan_rose at apple.com
Fri Jan 4 18:06:57 PST 2013


This is awesome. Would it be generally useful as a fixit for the protected scope problem in C++?

Jordan


On Jan 4, 2013, at 10:30 , Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:

> Author: akirtzidis
> Date: Fri Jan  4 12:30:08 2013
> New Revision: 171484
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=171484&view=rev
> Log:
> [arcmt] Adds brackets in case statements that "contain" initialization of retaining
> variable, thus emitting the "switch case is in protected scope" error.
> 
> rdar://12952016
> 
> Added:
>    cfe/trunk/lib/ARCMigrate/TransProtectedScope.cpp
>    cfe/trunk/test/ARCMT/protected-scope.m
>    cfe/trunk/test/ARCMT/protected-scope.m.result
> Modified:
>    cfe/trunk/lib/ARCMigrate/ARCMT.cpp
>    cfe/trunk/lib/ARCMigrate/Internals.h
>    cfe/trunk/lib/ARCMigrate/Transforms.cpp
>    cfe/trunk/lib/ARCMigrate/Transforms.h
>    cfe/trunk/test/ARCMT/checking.m
> 
> Modified: cfe/trunk/lib/ARCMigrate/ARCMT.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/ARCMT.cpp?rev=171484&r1=171483&r2=171484&view=diff
> ==============================================================================
> --- cfe/trunk/lib/ARCMigrate/ARCMT.cpp (original)
> +++ cfe/trunk/lib/ARCMigrate/ARCMT.cpp Fri Jan  4 12:30:08 2013
> @@ -39,8 +39,9 @@
>            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 @@
>   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 @@
>   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);
> 
> 
> Modified: cfe/trunk/lib/ARCMigrate/Internals.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/Internals.h?rev=171484&r1=171483&r2=171484&view=diff
> ==============================================================================
> --- cfe/trunk/lib/ARCMigrate/Internals.h (original)
> +++ cfe/trunk/lib/ARCMigrate/Internals.h Fri Jan  4 12:30:08 2013
> @@ -146,16 +146,20 @@
>   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; }
> 
> Added: cfe/trunk/lib/ARCMigrate/TransProtectedScope.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/TransProtectedScope.cpp?rev=171484&view=auto
> ==============================================================================
> --- cfe/trunk/lib/ARCMigrate/TransProtectedScope.cpp (added)
> +++ cfe/trunk/lib/ARCMigrate/TransProtectedScope.cpp Fri Jan  4 12:30:08 2013
> @@ -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;
> +  }
> +}
> 
> Modified: cfe/trunk/lib/ARCMigrate/Transforms.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/Transforms.cpp?rev=171484&r1=171483&r2=171484&view=diff
> ==============================================================================
> --- cfe/trunk/lib/ARCMigrate/Transforms.cpp (original)
> +++ cfe/trunk/lib/ARCMigrate/Transforms.cpp Fri Jan  4 12:30:08 2013
> @@ -564,6 +564,7 @@
>   }
>   MigrateCtx.addTraverser(new PropertyRewriteTraverser());
>   MigrateCtx.addTraverser(new BlockObjCVariableTraverser());
> +  MigrateCtx.addTraverser(new ProtectedScopeTraverser());
> 
>   MigrateCtx.traverse(pass.Ctx.getTranslationUnitDecl());
> }
> 
> Modified: cfe/trunk/lib/ARCMigrate/Transforms.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/Transforms.h?rev=171484&r1=171483&r2=171484&view=diff
> ==============================================================================
> --- cfe/trunk/lib/ARCMigrate/Transforms.h (original)
> +++ cfe/trunk/lib/ARCMigrate/Transforms.h Fri Jan  4 12:30:08 2013
> @@ -135,6 +135,11 @@
>   virtual void traverseBody(BodyContext &BodyCtx);
> };
> 
> +class ProtectedScopeTraverser : public ASTTraverser {
> +public:
> +  virtual void traverseBody(BodyContext &BodyCtx);
> +};
> +
> // GC transformations
> 
> class GCAttrsTraverser : public ASTTraverser {
> 
> Modified: cfe/trunk/test/ARCMT/checking.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/checking.m?rev=171484&r1=171483&r2=171484&view=diff
> ==============================================================================
> --- cfe/trunk/test/ARCMT/checking.m (original)
> +++ cfe/trunk/test/ARCMT/checking.m Fri Jan  4 12:30:08 2013
> @@ -178,13 +178,12 @@
> }
> 
> 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 @@
> 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;
>   }
> }
> 
> Added: cfe/trunk/test/ARCMT/protected-scope.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/protected-scope.m?rev=171484&view=auto
> ==============================================================================
> --- cfe/trunk/test/ARCMT/protected-scope.m (added)
> +++ cfe/trunk/test/ARCMT/protected-scope.m Fri Jan  4 12:30:08 2013
> @@ -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;
> +  }
> +}
> 
> Added: cfe/trunk/test/ARCMT/protected-scope.m.result
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/protected-scope.m.result?rev=171484&view=auto
> ==============================================================================
> --- cfe/trunk/test/ARCMT/protected-scope.m.result (added)
> +++ cfe/trunk/test/ARCMT/protected-scope.m.result Fri Jan  4 12:30:08 2013
> @@ -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;
> +  }
> +}
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list