From 2f43085b469c739ecd0a98c720aba254ceda018c Mon Sep 17 00:00:00 2001 From: Edwin Vane Date: Mon, 4 Mar 2013 16:35:04 +0000 Subject: [PATCH] Properly identify 'this' as range-based for container The Loop-Convert transform was mistransforming loops using 'this' implicitly. Fixed and added tests. Fixes PR15411. llvm-svn: 176436 --- .../cpp11-migrate/LoopConvert/LoopActions.cpp | 12 +++- .../test/cpp11-migrate/LoopConvert/iterator.cpp | 72 +++++++++++++++++++++- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.cpp b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.cpp index ed3bfc7..1a70070 100644 --- a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.cpp +++ b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopActions.cpp @@ -871,9 +871,15 @@ StringRef LoopFixer::checkDeferralsAndRejections(ASTContext *Context, return ""; } - StringRef ContainerString = - getStringFromRange(Context->getSourceManager(), Context->getLangOpts(), - ContainerExpr->getSourceRange()); + StringRef ContainerString; + if (isa(ContainerExpr->IgnoreParenImpCasts())) { + ContainerString = "this"; + } else { + ContainerString = getStringFromRange(Context->getSourceManager(), + Context->getLangOpts(), + ContainerExpr->getSourceRange()); + } + // In case someone is using an evil macro, reject this change. if (ContainerString.empty()) ++*RejectedChanges; diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp index 24981ed..a59a90f 100644 --- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp +++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp @@ -1,6 +1,8 @@ // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp // RUN: cpp11-migrate -loop-convert %t.cpp -- -I %S/Inputs // RUN: FileCheck -input-file=%t.cpp %s +// RUN: cpp11-migrate -loop-convert %t.cpp -risk=risky -- -I %S/Inputs +// RUN: FileCheck -check-prefix=RISKY -input-file=%t.cpp %s #include "structures.h" @@ -99,5 +101,73 @@ void f() { } // CHECK: for ({{[a-zA-Z_ ]*&? ?}}[[VAR:[a-z_]+]] : intmap) // CHECK-NEXT: printf("intmap[%d] = %d", [[VAR]].first, [[VAR]].second); - } + +// Tests to ensure that an implicit 'this' is picked up as the container. +// If member calls are made to 'this' within the loop, the transform becomes +// risky as these calls may affect state that affects the loop. +class C { +public: + typedef MutableVal *iterator; + typedef const MutableVal *const_iterator; + + iterator begin(); + iterator end(); + const_iterator begin() const; + const_iterator end() const; + + void doSomething(); + void doSomething() const; + + void doLoop() { + for (iterator I = begin(), E = end(); I != E; ++I) { + // CHECK: for (auto & elem : *this) { + } + for (iterator I = C::begin(), E = C::end(); I != E; ++I) { + // CHECK: for (auto & elem : *this) { + } + for (iterator I = begin(), E = end(); I != E; ++I) { + // CHECK: for (iterator I = begin(), E = end(); I != E; ++I) { + // RISKY: for (auto & elem : *this) { + doSomething(); + } + for (iterator I = begin(); I != end(); ++I) { + // CHECK: for (auto & elem : *this) { + } + for (iterator I = begin(); I != end(); ++I) { + // CHECK: for (iterator I = begin(); I != end(); ++I) { + // RISKY: for (auto & elem : *this) { + doSomething(); + } + } + + void doLoop() const { + for (const_iterator I = begin(), E = end(); I != E; ++I) { + // CHECK: for (auto & elem : *this) { + } + for (const_iterator I = C::begin(), E = C::end(); I != E; ++I) { + // CHECK: for (auto & elem : *this) { + } + for (const_iterator I = begin(), E = end(); I != E; ++I) { + // CHECK: for (const_iterator I = begin(), E = end(); I != E; ++I) { + // RISKY: for (auto & elem : *this) { + doSomething(); + } + } +}; + +class C2 { +public: + typedef MutableVal *iterator; + + iterator begin() const; + iterator end() const; + + void doLoop() { + // The implicit 'this' will have an Implicit cast to const C2* wrapped + // around it. Make sure the replacement still happens. + for (iterator I = begin(), E = end(); I != E; ++I) { + // CHECK: for (auto & elem : *this) { + } + } +}; -- 2.7.4