[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