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 <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
