angelgarcia created this revision.
angelgarcia added a reviewer: klimek.
angelgarcia added subscribers: alexfh, cfe-commits.
This fixes https://llvm.org/bugs/show_bug.cgi?id=17716.
http://reviews.llvm.org/D13342
Files:
clang-tidy/modernize/LoopConvertCheck.cpp
clang-tidy/modernize/LoopConvertCheck.h
test/clang-tidy/modernize-loop-convert-extra.cpp
Index: test/clang-tidy/modernize-loop-convert-extra.cpp
===================================================================
--- test/clang-tidy/modernize-loop-convert-extra.cpp
+++ test/clang-tidy/modernize-loop-convert-extra.cpp
@@ -53,7 +53,7 @@
// 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 @@
}
// 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 @@
}
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
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 } ) } };
Index: clang-tidy/modernize/LoopConvertCheck.h
===================================================================
--- clang-tidy/modernize/LoopConvertCheck.h
+++ clang-tidy/modernize/LoopConvertCheck.h
@@ -34,6 +34,8 @@
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,
Index: clang-tidy/modernize/LoopConvertCheck.cpp
===================================================================
--- clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tidy/modernize/LoopConvertCheck.cpp
@@ -442,6 +442,30 @@
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 trailing whitespaces, by extending the range until the beginning of
+/// the next instruction.
+///
+/// FIXME: if 'next_instruction' is a closing brace ('}'), after the replacement
+/// it will be over-indented. But then, who would declare an alias and do
+/// nothing with it before it goes out of the scope?
+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 @@
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 @@
// 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(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
