Skip to content

Commit

Permalink
Don't use "auto" on loops over fundamental types in modernize-loop-co…
Browse files Browse the repository at this point in the history
…nvert.

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
  • Loading branch information
angargo committed Oct 22, 2015
1 parent b89658f commit d8336f3
Show file tree
Hide file tree
Showing 7 changed files with 166 additions and 115 deletions.
37 changes: 18 additions & 19 deletions clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
Expand Up @@ -405,7 +405,7 @@ static bool containerIsConst(const Expr *ContainerExpr, bool Dereference) {

LoopConvertCheck::RangeDescriptor::RangeDescriptor()
: ContainerNeedsDereference(false), DerefByConstRef(false),
DerefByValue(false), IsTriviallyCopyable(false) {}
DerefByValue(false) {}

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

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

// If the new variable name is from the aliased variable, then the reference
// type for the new variable should only be used if the aliased variable was
// declared as a reference.
bool IsTriviallyCopyable =
!Descriptor.ElemType.isNull() &&
Descriptor.ElemType.isTriviallyCopyableType(*Context);
bool UseCopy =
CanCopy &&
((VarNameFromAlias && !AliasVarIsRef) ||
(Descriptor.DerefByConstRef && Descriptor.IsTriviallyCopyable));
CanCopy && ((VarNameFromAlias && !AliasVarIsRef) ||
(Descriptor.DerefByConstRef && IsTriviallyCopyable));

if (!UseCopy) {
if (Descriptor.DerefByConstRef) {
AutoType =
Context->getLValueReferenceType(Context->getConstType(AutoType));
Type = Context->getLValueReferenceType(Context->getConstType(Type));
} else if (Descriptor.DerefByValue) {
if (!Descriptor.IsTriviallyCopyable)
AutoType = Context->getRValueReferenceType(AutoType);
if (!IsTriviallyCopyable)
Type = Context->getRValueReferenceType(Type);
} else {
AutoType = Context->getLValueReferenceType(AutoType);
Type = Context->getLValueReferenceType(Type);
}
}

StringRef MaybeDereference = Descriptor.ContainerNeedsDereference ? "*" : "";
std::string TypeString = AutoType.getAsString();
std::string TypeString = Type.getAsString(getLangOpts());
std::string Range = ("(" + TypeString + " " + VarName + " : " +
MaybeDereference + Descriptor.ContainerString + ")")
.str();
Expand Down Expand Up @@ -633,7 +636,7 @@ void LoopConvertCheck::getArrayLoopQualifiers(ASTContext *Context,
}
Type = Type->getPointeeType();
}
Descriptor.IsTriviallyCopyable = Type.isTriviallyCopyableType(*Context);
Descriptor.ElemType = Type;
}
}

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

Descriptor.DerefByConstRef = ValueType.isConstQualified();
Descriptor.IsTriviallyCopyable =
ValueType.isTriviallyCopyableType(*Context);
Descriptor.ElemType = ValueType;
} else {
// By nature of the matcher this case is triggered only for built-in
// iterator types (i.e. pointers).
Expand All @@ -676,9 +677,7 @@ void LoopConvertCheck::getIteratorLoopQualifiers(ASTContext *Context,
// We test for const qualification of the pointed-at type.
Descriptor.DerefByConstRef =
CanonicalInitVarType->getPointeeType().isConstQualified();
Descriptor.IsTriviallyCopyable =
CanonicalInitVarType->getPointeeType().isTriviallyCopyableType(
*Context);
Descriptor.ElemType = CanonicalInitVarType->getPointeeType();
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
Expand Up @@ -30,8 +30,8 @@ class LoopConvertCheck : public ClangTidyCheck {
bool ContainerNeedsDereference;
bool DerefByConstRef;
bool DerefByValue;
bool IsTriviallyCopyable;
std::string ContainerString;
QualType ElemType;
};

void getAliasRange(SourceManager &SM, SourceRange &DeclRange);
Expand Down

0 comments on commit d8336f3

Please sign in to comment.