Skip to content

Commit d8336f3

Browse files
committed
Don't use "auto" on loops over fundamental types in modernize-loop-convert.
Summary: using "auto" on a loop that iterates over ints is kind of an overkill. Use the real type name instead. Reviewers: klimek Subscribers: alexfh, cfe-commits Differential Revision: http://reviews.llvm.org/D13982 llvm-svn: 251015
1 parent b89658f commit d8336f3

File tree

7 files changed

+166
-115
lines changed

7 files changed

+166
-115
lines changed

clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ static bool containerIsConst(const Expr *ContainerExpr, bool Dereference) {
405405

406406
LoopConvertCheck::RangeDescriptor::RangeDescriptor()
407407
: ContainerNeedsDereference(false), DerefByConstRef(false),
408-
DerefByValue(false), IsTriviallyCopyable(false) {}
408+
DerefByValue(false) {}
409409

410410
LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context)
411411
: ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo),
@@ -554,30 +554,33 @@ void LoopConvertCheck::doConversion(
554554
// Now, we need to construct the new range expression.
555555
SourceRange ParenRange(Loop->getLParenLoc(), Loop->getRParenLoc());
556556

557-
QualType AutoType = Context->getAutoDeductType();
557+
QualType Type = Context->getAutoDeductType();
558+
if (!Descriptor.ElemType.isNull() && Descriptor.ElemType->isFundamentalType())
559+
Type = Descriptor.ElemType.getUnqualifiedType();
558560

559561
// If the new variable name is from the aliased variable, then the reference
560562
// type for the new variable should only be used if the aliased variable was
561563
// declared as a reference.
564+
bool IsTriviallyCopyable =
565+
!Descriptor.ElemType.isNull() &&
566+
Descriptor.ElemType.isTriviallyCopyableType(*Context);
562567
bool UseCopy =
563-
CanCopy &&
564-
((VarNameFromAlias && !AliasVarIsRef) ||
565-
(Descriptor.DerefByConstRef && Descriptor.IsTriviallyCopyable));
568+
CanCopy && ((VarNameFromAlias && !AliasVarIsRef) ||
569+
(Descriptor.DerefByConstRef && IsTriviallyCopyable));
566570

567571
if (!UseCopy) {
568572
if (Descriptor.DerefByConstRef) {
569-
AutoType =
570-
Context->getLValueReferenceType(Context->getConstType(AutoType));
573+
Type = Context->getLValueReferenceType(Context->getConstType(Type));
571574
} else if (Descriptor.DerefByValue) {
572-
if (!Descriptor.IsTriviallyCopyable)
573-
AutoType = Context->getRValueReferenceType(AutoType);
575+
if (!IsTriviallyCopyable)
576+
Type = Context->getRValueReferenceType(Type);
574577
} else {
575-
AutoType = Context->getLValueReferenceType(AutoType);
578+
Type = Context->getLValueReferenceType(Type);
576579
}
577580
}
578581

579582
StringRef MaybeDereference = Descriptor.ContainerNeedsDereference ? "*" : "";
580-
std::string TypeString = AutoType.getAsString();
583+
std::string TypeString = Type.getAsString(getLangOpts());
581584
std::string Range = ("(" + TypeString + " " + VarName + " : " +
582585
MaybeDereference + Descriptor.ContainerString + ")")
583586
.str();
@@ -633,7 +636,7 @@ void LoopConvertCheck::getArrayLoopQualifiers(ASTContext *Context,
633636
}
634637
Type = Type->getPointeeType();
635638
}
636-
Descriptor.IsTriviallyCopyable = Type.isTriviallyCopyableType(*Context);
639+
Descriptor.ElemType = Type;
637640
}
638641
}
639642

@@ -654,8 +657,7 @@ void LoopConvertCheck::getIteratorLoopQualifiers(ASTContext *Context,
654657
// If the dereference operator returns by value then test for the
655658
// canonical const qualification of the init variable type.
656659
Descriptor.DerefByConstRef = CanonicalInitVarType.isConstQualified();
657-
Descriptor.IsTriviallyCopyable =
658-
DerefByValueType->isTriviallyCopyableType(*Context);
660+
Descriptor.ElemType = *DerefByValueType;
659661
} else {
660662
if (const auto *DerefType =
661663
Nodes.getNodeAs<QualType>(DerefByRefResultName)) {
@@ -665,8 +667,7 @@ void LoopConvertCheck::getIteratorLoopQualifiers(ASTContext *Context,
665667
auto ValueType = DerefType->getNonReferenceType();
666668

667669
Descriptor.DerefByConstRef = ValueType.isConstQualified();
668-
Descriptor.IsTriviallyCopyable =
669-
ValueType.isTriviallyCopyableType(*Context);
670+
Descriptor.ElemType = ValueType;
670671
} else {
671672
// By nature of the matcher this case is triggered only for built-in
672673
// iterator types (i.e. pointers).
@@ -676,9 +677,7 @@ void LoopConvertCheck::getIteratorLoopQualifiers(ASTContext *Context,
676677
// We test for const qualification of the pointed-at type.
677678
Descriptor.DerefByConstRef =
678679
CanonicalInitVarType->getPointeeType().isConstQualified();
679-
Descriptor.IsTriviallyCopyable =
680-
CanonicalInitVarType->getPointeeType().isTriviallyCopyableType(
681-
*Context);
680+
Descriptor.ElemType = CanonicalInitVarType->getPointeeType();
682681
}
683682
}
684683
}

clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ class LoopConvertCheck : public ClangTidyCheck {
3030
bool ContainerNeedsDereference;
3131
bool DerefByConstRef;
3232
bool DerefByValue;
33-
bool IsTriviallyCopyable;
3433
std::string ContainerString;
34+
QualType ElemType;
3535
};
3636

3737
void getAliasRange(SourceManager &SM, SourceRange &DeclRange);

0 commit comments

Comments
 (0)