Certainly! http://llvm.org/bugs/show_bug.cgi?id=15500
Thanks! ~Aaron On Tue, Mar 12, 2013 at 2:42 PM, Argyrios Kyrtzidis <[email protected]> wrote: > On Mar 12, 2013, at 11:32 AM, Aaron Ballman <[email protected]> wrote: > > Understandable. Since it's been broken since Jan 7, if it remains > broken another week, we can probably survive somehow. ;-) But > hopefully it stays on your radar. > > > Could you please file a bug report, we should track this in bugzilla as > well. > > > Thanks! > > ~Aaron > > On Tue, Mar 12, 2013 at 2:19 PM, Argyrios Kyrtzidis <[email protected]> > wrote: > > On Mar 12, 2013, at 10:48 AM, Aaron Ballman <[email protected]> wrote: > > Have you had the chance to look into this? It's still unresolved for > me, and causes the MSVC 11 debug builds to fail. > > > No sorry. This week is pretty busy not sure I can look into it this week. > > -Argyrios > > > Thanks! > > ~Aaron > > On Mon, Feb 18, 2013 at 10:26 PM, Argyrios Kyrtzidis <[email protected]> > wrote: > > Thanks for letting me know Aaron, I'll look into it. > > -Argyrios > > On Feb 18, 2013, at 5:59 PM, Aaron Ballman <[email protected]> wrote: > > Sorry about the incredible "lateness" of this review, but the error > has been bugging me for a while. > > On Mon, Jan 7, 2013 at 7:58 PM, Argyrios Kyrtzidis <[email protected]> > wrote: > > Author: akirtzidis > Date: Mon Jan 7 18:58:25 2013 > New Revision: 171828 > > URL: http://llvm.org/viewvc/llvm-project?rev=171828&view=rev > Log: > [arcmt] Follow-up for r171484; make sure when adding brackets enclosing case > statements, > that the case does not "contain" a declaration that is referenced "outside" > of it, > otherwise we will emit un-compilable code. > > Modified: > cfe/trunk/lib/ARCMigrate/TransProtectedScope.cpp > cfe/trunk/test/ARCMT/checking.m > cfe/trunk/test/ARCMT/protected-scope.m > cfe/trunk/test/ARCMT/protected-scope.m.result > > Modified: cfe/trunk/lib/ARCMigrate/TransProtectedScope.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/TransProtectedScope.cpp?rev=171828&r1=171827&r2=171828&view=diff > ============================================================================== > --- cfe/trunk/lib/ARCMigrate/TransProtectedScope.cpp (original) > +++ cfe/trunk/lib/ARCMigrate/TransProtectedScope.cpp Mon Jan 7 18:58:25 > 2013 > @@ -15,6 +15,7 @@ > #include "Transforms.h" > #include "Internals.h" > #include "clang/Sema/SemaDiagnostic.h" > +#include "clang/AST/ASTContext.h" > > using namespace clang; > using namespace arcmt; > @@ -22,26 +23,58 @@ > > namespace { > > +class LocalRefsCollector : public RecursiveASTVisitor<LocalRefsCollector> { > + SmallVectorImpl<DeclRefExpr *> &Refs; > + > +public: > + LocalRefsCollector(SmallVectorImpl<DeclRefExpr *> &refs) > + : Refs(refs) { } > + > + bool VisitDeclRefExpr(DeclRefExpr *E) { > + if (ValueDecl *D = E->getDecl()) > + if (D->getDeclContext()->getRedeclContext()->isFunctionOrMethod()) > + Refs.push_back(E); > + return true; > + } > +}; > + > struct CaseInfo { > SwitchCase *SC; > SourceRange Range; > - bool FixedBypass; > + enum { > + St_Unchecked, > + St_CannotFix, > + St_Fixed > + } State; > > - CaseInfo() : SC(0), FixedBypass(false) {} > + CaseInfo() : SC(0), State(St_Unchecked) {} > CaseInfo(SwitchCase *S, SourceRange Range) > - : SC(S), Range(Range), FixedBypass(false) {} > + : SC(S), Range(Range), State(St_Unchecked) {} > }; > > class CaseCollector : public RecursiveASTVisitor<CaseCollector> { > + ParentMap &PMap; > llvm::SmallVectorImpl<CaseInfo> &Cases; > > public: > - CaseCollector(llvm::SmallVectorImpl<CaseInfo> &Cases) > - : Cases(Cases) { } > + CaseCollector(ParentMap &PMap, llvm::SmallVectorImpl<CaseInfo> &Cases) > + : PMap(PMap), Cases(Cases) { } > > bool VisitSwitchStmt(SwitchStmt *S) { > - SourceLocation NextLoc = S->getLocEnd(); > SwitchCase *Curr = S->getSwitchCaseList(); > + if (!Curr) > + return true; > + Stmt *Parent = getCaseParent(Curr); > + Curr = Curr->getNextSwitchCase(); > + // Make sure all case statements are in the same scope. > + while (Curr) { > + if (getCaseParent(Curr) != Parent) > + return true; > + Curr = Curr->getNextSwitchCase(); > + } > + > + SourceLocation NextLoc = S->getLocEnd(); > + Curr = S->getSwitchCaseList(); > // We iterate over case statements in reverse source-order. > while (Curr) { > Cases.push_back(CaseInfo(Curr,SourceRange(Curr->getLocStart(), > NextLoc))); > @@ -50,69 +83,114 @@ > } > return true; > } > -}; > > -} // anonymous namespace > + Stmt *getCaseParent(SwitchCase *S) { > + Stmt *Parent = PMap.getParent(S); > + while (Parent && (isa<SwitchCase>(Parent) || isa<LabelStmt>(Parent))) > + Parent = PMap.getParent(Parent); > + return Parent; > + } > +}; > > -static bool isInRange(FullSourceLoc Loc, SourceRange R) { > - return !Loc.isBeforeInTranslationUnitThan(R.getBegin()) && > - Loc.isBeforeInTranslationUnitThan(R.getEnd()); > -} > +class ProtectedScopeFixer { > + MigrationPass &Pass; > + SourceManager &SM; > + SmallVector<CaseInfo, 16> Cases; > + SmallVector<DeclRefExpr *, 16> LocalRefs; > > -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; > +public: > + ProtectedScopeFixer(BodyContext &BodyCtx) > + : Pass(BodyCtx.getMigrationContext().Pass), > + SM(Pass.Ctx.getSourceManager()) { > + > + CaseCollector(BodyCtx.getParentMap(), Cases) > + .TraverseStmt(BodyCtx.getTopStmt()); > + LocalRefsCollector(LocalRefs).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); > + continue; > > > The use of iterators here is problematic because > handleProtectedScopeError can invalidate the iterators. This is > causing a failed assertion in debug build of MSVC11 where there are > checked iterators involved. > > From what I am seeing, handleProtectedScopeError has a Transaction > object, which on destruction winds up calling > CapturedDiagList::clearDiagnostic, and that calls erase on the list, > which invalidates the iterators. Attempting to perform the comparison > then fires the assert. > > I'm not entirely certain of the best way to solve this issue, but I > figured I would mention that it causes problems. We have a test case > already demonstrating the issue with test\ARCMT\protected-scope.m > > ~Aaron > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
