maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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

llvm-svn: 249006
This commit is contained in:
Angel Garcia Gomez 2015-10-01 13:08:21 +00:00
parent 41106188a4
commit 90bf895aa3
3 changed files with 50 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
View File

@ -442,6 +442,30 @@ void LoopConvertCheck::registerMatchers(MatchFinder *Finder) {
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(

2
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
View File

@ -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,

30
clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp
View File

@ -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 } ) } };