[clang-tools-extra] r249006 - Prevent loop-convert from leaving empty lines after removing an alias declaration.
Angel Garcia Gomez via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 1 06:08:22 PDT 2015
Author: angelgarcia
Date: Thu Oct 1 08:08:21 2015
New Revision: 249006
URL: http://llvm.org/viewvc/llvm-project?rev=249006&view=rev
Log:
Prevent loop-convert from leaving empty lines after removing an alias declaration.
Summary: This fixes https://llvm.org/bugs/show_bug.cgi?id=17716.
Reviewers: klimek
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D13342
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=249006&r1=249005&r2=249006&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Thu Oct 1 08:08:21 2015
@@ -442,6 +442,30 @@ void LoopConvertCheck::registerMatchers(
Finder->addMatcher(makePseudoArrayLoopMatcher(), this);
}
+/// \brief Given the range of a single declaration, such as:
+/// \code
+/// unsigned &ThisIsADeclarationThatCanSpanSeveralLinesOfCode =
+/// InitializationValues[I];
+/// next_instruction;
+/// \endcode
+/// Finds the range that has to be erased to remove this declaration without
+/// leaving empty lines, by extending the range until the beginning of the
+/// next instruction.
+///
+/// We need to delete a potential newline after the deleted alias, as
+/// clang-format will leave empty lines untouched. For all other formatting we
+/// rely on clang-format to fix it.
+void LoopConvertCheck::getAliasRange(SourceManager &SM, SourceRange &Range) {
+ bool Invalid = false;
+ const char *TextAfter =
+ SM.getCharacterData(Range.getEnd().getLocWithOffset(1), &Invalid);
+ if (Invalid)
+ return;
+ unsigned Offset = std::strspn(TextAfter, " \t\r\n");
+ Range =
+ SourceRange(Range.getBegin(), Range.getEnd().getLocWithOffset(Offset));
+}
+
/// \brief Computes the changes needed to convert a given for loop, and
/// applies them.
void LoopConvertCheck::doConversion(
@@ -460,7 +484,7 @@ void LoopConvertCheck::doConversion(
AliasVarIsRef = AliasVar->getType()->isReferenceType();
// We keep along the entire DeclStmt to keep the correct range here.
- const SourceRange &ReplaceRange = AliasDecl->getSourceRange();
+ SourceRange ReplaceRange = AliasDecl->getSourceRange();
std::string ReplacementText;
if (AliasUseRequired) {
@@ -470,6 +494,9 @@ void LoopConvertCheck::doConversion(
// in a for loop's init clause. Need to put this ';' back while removing
// the declaration of the alias variable. This is probably a bug.
ReplacementText = ";";
+ } else {
+ // Avoid leaving empty lines or trailing whitespaces.
+ getAliasRange(Context->getSourceManager(), ReplaceRange);
}
Diag << FixItHint::CreateReplacement(
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h?rev=249006&r1=249005&r2=249006&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h Thu Oct 1 08:08:21 2015
@@ -34,6 +34,8 @@ private:
std::string ContainerString;
};
+ void getAliasRange(SourceManager &SM, SourceRange &DeclRange);
+
void doConversion(ASTContext *Context, const VarDecl *IndexVar,
const VarDecl *MaybeContainer, const UsageResult &Usages,
const DeclStmt *AliasDecl, bool AliasUseRequired,
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp?rev=249006&r1=249005&r2=249006&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp Thu Oct 1 08:08:21 2015
@@ -53,7 +53,7 @@ void aliasing() {
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & T : Arr)
// CHECK-FIXES-NOT: Val &{{[a-z_]+}} =
- // CHECK-FIXES: {}
+ // CHECK-FIXES-NEXT: {}
// CHECK-FIXES-NEXT: int Y = T.X;
// The container was not only used to initialize a temporary loop variable for
@@ -89,7 +89,7 @@ void aliasing() {
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & T : V)
- // CHECK-FIXES: {}
+ // CHECK-FIXES-NEXT: {}
// CHECK-FIXES-NEXT: int Y = T.X;
// The same with a call to at()
@@ -100,7 +100,7 @@ void aliasing() {
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & T : *Pv)
- // CHECK-FIXES: {}
+ // CHECK-FIXES-NEXT: {}
// CHECK-FIXES-NEXT: int Y = T.X;
for (int I = 0; I < N; ++I) {
@@ -166,8 +166,17 @@ void aliasing() {
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & Elem : IntArr)
// CHECK-FIXES-NEXT: IntRef Int(Elem);
-}
+ // Ensure that removing the alias doesn't leave empty lines behind.
+ for (int I = 0; I < N; ++I) {
+ auto &X = IntArr[I];
+ X = 0;
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & X : IntArr) {
+ // CHECK-FIXES-NEXT: {{^ X = 0;$}}
+ // CHECK-FIXES-NEXT: {{^ }$}}
+}
void refs_and_vals() {
// The following tests check that the transform correctly preserves the
@@ -186,7 +195,7 @@ void refs_and_vals() {
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto Alias : S_const)
// CHECK-FIXES-NOT: MutableVal {{[a-z_]+}} =
- // CHECK-FIXES: {}
+ // CHECK-FIXES-NEXT: {}
// CHECK-FIXES-NEXT: Alias.X = 0;
for (S::iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) {
@@ -197,7 +206,7 @@ void refs_and_vals() {
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto Alias : Ss)
// CHECK-FIXES-NOT: MutableVal {{[a-z_]+}} =
- // CHECK-FIXES: {}
+ // CHECK-FIXES-NEXT: {}
// CHECK-FIXES-NEXT: Alias.X = 0;
for (S::iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) {
@@ -208,7 +217,7 @@ void refs_and_vals() {
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & Alias : Ss)
// CHECK-FIXES-NOT: MutableVal &{{[a-z_]+}} =
- // CHECK-FIXES: {}
+ // CHECK-FIXES-NEXT: {}
// CHECK-FIXES-NEXT: Alias.X = 0;
dependent<int> Dep, Other;
@@ -863,7 +872,7 @@ void implicitCapture() {
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto R5 : Arr)
// CHECK-FIXES-NEXT: auto G5 = [&]()
- // CHECK-FIXES: int J5 = 8 + R5;
+ // CHECK-FIXES-NEXT: int J5 = 8 + R5;
// Alias by reference.
for (int I = 0; I < N; ++I) {
@@ -875,7 +884,7 @@ void implicitCapture() {
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & R6 : Arr)
// CHECK-FIXES-NEXT: auto G6 = [&]()
- // CHECK-FIXES: int J6 = -1 + R6;
+ // CHECK-FIXES-NEXT: int J6 = -1 + R6;
}
void iterators() {
@@ -953,7 +962,8 @@ void f() {
E Ee{ { { g( { Array[I] } ) } } };
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: int A{ Elem };
+ // CHECK-FIXES: for (auto & Elem : Array)
+ // CHECK-FIXES-NEXT: int A{ Elem };
// CHECK-FIXES-NEXT: int B{ g(Elem) };
// CHECK-FIXES-NEXT: int C{ g( { Elem } ) };
// CHECK-FIXES-NEXT: D Dd{ { g( { Elem } ) } };
More information about the cfe-commits
mailing list